[vlc-commits] playlist: use search trees for ID and input to item mapping
Rémi Denis-Courmont
git at videolan.org
Fri Nov 18 21:40:59 CET 2016
vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Fri Nov 18 21:40:54 2016 +0200| [f2a6855990c71b28453a6083cc6e37b16b73faac] | committer: Rémi Denis-Courmont
playlist: use search trees for ID and input to item mapping
Regarding input item look-ups, this reduces asymptotic complexity from
linear to logarithmic time.
Regarding ID look-ups, this reduces insertion and deletion time to
logarithmic. Previously it degraded to linear time because of memcpy()
and memmove() in ARRAY_APPEND and ARRAY_REMOVE macros.
This removes the "all_items" array, and its missing error handlers.
Finally, this adds support for allocating more than INT_MAX items
during the entire lifetime of the VLC instance. (The maximum number of
_concurrent_ items is still INT_MAX, but memory would probably run out
before that is reached.)
Note: Item deletion still requires linear time. And playlist deletion
still consequently requires quadractic time because of the "current"
array.
> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=f2a6855990c71b28453a6083cc6e37b16b73faac
---
src/playlist/engine.c | 7 +-
src/playlist/item.c | 139 +++++++++++++++++++++++++++++++++++----
src/playlist/playlist_internal.h | 6 +-
src/playlist/search.c | 46 -------------
src/playlist/tree.c | 7 +-
5 files changed, 135 insertions(+), 70 deletions(-)
diff --git a/src/playlist/engine.c b/src/playlist/engine.c
index dbb2723..dc5eff2 100644
--- a/src/playlist/engine.c
+++ b/src/playlist/engine.c
@@ -209,6 +209,10 @@ playlist_t *playlist_Create( vlc_object_t *p_parent )
assert( offsetof( playlist_private_t, public_data ) == 0 );
p_playlist = &p->public_data;
+
+ p->input_tree = NULL;
+ p->id_tree = NULL;
+
TAB_INIT( pl_priv(p_playlist)->i_sds, pl_priv(p_playlist)->pp_sds );
VariablesInit( p_playlist );
@@ -221,7 +225,6 @@ playlist_t *playlist_Create( vlc_object_t *p_parent )
pl_priv(p_playlist)->p_input = NULL;
ARRAY_INIT( p_playlist->items );
- ARRAY_INIT( p->all_items );
ARRAY_INIT( p_playlist->current );
p_playlist->i_current_index = 0;
@@ -332,8 +335,6 @@ void playlist_Destroy( playlist_t *p_playlist )
vlc_cond_destroy( &p_sys->signal );
vlc_mutex_destroy( &p_sys->lock );
- ARRAY_RESET( p_sys->all_items );
-
ARRAY_RESET( p_playlist->items );
ARRAY_RESET( p_playlist->current );
diff --git a/src/playlist/item.c b/src/playlist/item.c
index 4caaa34..4231cb9 100644
--- a/src/playlist/item.c
+++ b/src/playlist/item.c
@@ -25,8 +25,13 @@
# include "config.h"
#endif
-#include <vlc_common.h>
#include <assert.h>
+#include <limits.h>
+#ifdef HAVE_SEARCH_H
+# include <search.h>
+#endif
+
+#include <vlc_common.h>
#include <vlc_playlist.h>
#include <vlc_rand.h>
#include "playlist_internal.h"
@@ -263,23 +268,41 @@ static void uninstall_input_item_observer( playlist_item_t * p_item )
input_item_changed, p_item );
}
+static int playlist_ItemCmpId( const void *a, const void *b )
+{
+ const playlist_item_t *pa = a, *pb = b;
+
+ /* ID are between 1 and INT_MAX, this cannot overflow. */
+ return pa->i_id - pb->i_id;
+}
+
+static int playlist_ItemCmpInput( const void *a, const void *b )
+{
+ const playlist_item_t *pa = a, *pb = b;
+
+ if( pa->p_input == pb->p_input )
+ return 0;
+ return (((uintptr_t)pa->p_input) > ((uintptr_t)pb->p_input))
+ ? +1 : -1;
+}
+
/*****************************************************************************
* Playlist item creation
*****************************************************************************/
playlist_item_t *playlist_ItemNewFromInput( playlist_t *p_playlist,
input_item_t *p_input )
{
- playlist_item_t* p_item = malloc( sizeof( playlist_item_t ) );
- if( !p_item )
+ playlist_private_t *p = pl_priv(p_playlist);
+ playlist_item_t **pp, *p_item;
+
+ p_item = malloc( sizeof( playlist_item_t ) );
+ if( unlikely(p_item == NULL) )
return NULL;
assert( p_input );
p_item->p_input = p_input;
- vlc_gc_incref( p_item->p_input );
-
- p_item->i_id = ++pl_priv(p_playlist)->i_last_playlist_id;
-
+ p_item->i_id = p->i_last_playlist_id;
p_item->p_parent = NULL;
p_item->i_children = -1;
p_item->pp_children = NULL;
@@ -287,9 +310,44 @@ playlist_item_t *playlist_ItemNewFromInput( playlist_t *p_playlist,
p_item->i_flags = 0;
p_item->p_playlist = p_playlist;
- install_input_item_observer( p_item );
+ PL_ASSERT_LOCKED;
+
+ do /* Find an unused ID for the item */
+ {
+ if( unlikely(p_item->i_id == INT_MAX) )
+ p_item->i_id = 0;
+
+ p_item->i_id++;
+
+ if( unlikely(p_item->i_id == p->i_last_playlist_id) )
+ goto error; /* All IDs taken */
+
+ pp = tsearch( p_item, &p->id_tree, playlist_ItemCmpId );
+ if( unlikely(pp == NULL) )
+ goto error;
+
+ assert( (*pp)->i_id == p_item->i_id );
+ assert( (*pp) == p_item || (*pp)->p_input != p_input );
+ }
+ while( p_item != *pp );
+ pp = tsearch( p_item, &p->input_tree, playlist_ItemCmpInput );
+ if( unlikely(pp == NULL) )
+ {
+ tdelete( p_item, &p->id_tree, playlist_ItemCmpId );
+ goto error;
+ }
+ /* Same input item cannot be inserted twice. */
+ assert( p_item == *pp );
+
+ p->i_last_playlist_id = p_item->i_id;
+ vlc_gc_incref( p_item->p_input );
+ install_input_item_observer( p_item );
return p_item;
+
+error:
+ free( p_item );
+ return NULL;
}
/***************************************************************************
@@ -301,19 +359,75 @@ playlist_item_t *playlist_ItemNewFromInput( playlist_t *p_playlist,
*
* \param p_item item to delete
*/
-void playlist_ItemRelease( playlist_item_t *p_item )
+void playlist_ItemRelease( playlist_t *p_playlist, playlist_item_t *p_item )
{
- /* For the assert */
- playlist_t *p_playlist = p_item->p_playlist;
+ playlist_private_t *p = pl_priv(p_playlist);
+
PL_ASSERT_LOCKED;
uninstall_input_item_observer( p_item );
- free( p_item->pp_children );
vlc_gc_decref( p_item->p_input );
+
+ tdelete( p_item, &p->input_tree, playlist_ItemCmpInput );
+ tdelete( p_item, &p->id_tree, playlist_ItemCmpId );
+ free( p_item->pp_children );
free( p_item );
}
/**
+ * Finds a playlist item by ID.
+ *
+ * Searches for a playlist item with the given ID.
+ *
+ * \note The playlist must be locked, and the result is only valid until the
+ * playlist is unlocked.
+ *
+ * \warning If an item with the given ID is deleted, it is unlikely but
+ * possible that another item will get the same ID. This can result in
+ * mismatches.
+ * Where holding a reference to an input item is a viable option, then
+ * playlist_ItemGetByInput() should be used instead - to avoid this issue.
+ *
+ * @param p_playlist the playlist
+ * @param id ID to look for
+ * @return the matching item or NULL if not found
+ */
+playlist_item_t *playlist_ItemGetById( playlist_t *p_playlist , int id )
+{
+ playlist_private_t *p = pl_priv(p_playlist);
+ playlist_item_t key, **pp;
+
+ PL_ASSERT_LOCKED;
+ key.i_id = id;
+ pp = tfind( &key, &p->id_tree, playlist_ItemCmpId );
+ return (pp != NULL) ? *pp : NULL;
+}
+
+/**
+ * Finds a playlist item by input item.
+ *
+ * Searches for a playlist item for the given input item.
+ *
+ * \note The playlist must be locked, and the result is only valid until the
+ * playlist is unlocked.
+ *
+ * \param p_playlist the playlist
+ * \param item input item to look for
+ * \return the playlist item or NULL on failure
+ */
+playlist_item_t *playlist_ItemGetByInput( playlist_t * p_playlist,
+ const input_item_t *item )
+{
+ playlist_private_t *p = pl_priv(p_playlist);
+ playlist_item_t key, **pp;
+
+ PL_ASSERT_LOCKED;
+ key.p_input = (input_item_t *)item;
+ pp = tfind( &key, &p->input_tree, playlist_ItemCmpInput );
+ return (pp != NULL) ? *pp : NULL;
+}
+
+/**
* Clear the playlist
*
* \param p_playlist playlist object
@@ -719,7 +833,6 @@ static void AddItem( playlist_t *p_playlist, playlist_item_t *p_item,
{
PL_ASSERT_LOCKED;
ARRAY_APPEND(p_playlist->items, p_item);
- ARRAY_APPEND(pl_priv(p_playlist)->all_items, p_item);
playlist_NodeInsert( p_playlist, p_item, p_node, i_pos );
playlist_SendAddNotify( p_playlist, p_item,
diff --git a/src/playlist/playlist_internal.h b/src/playlist/playlist_internal.h
index efcd587..1395466 100644
--- a/src/playlist/playlist_internal.h
+++ b/src/playlist/playlist_internal.h
@@ -49,7 +49,9 @@ typedef struct playlist_private_t
playlist_t public_data;
struct intf_thread_t *interface; /**< Linked-list of interfaces */
- playlist_item_array_t all_items; /**< Array of items and nodes */
+ void *input_tree; /**< Search tree for input item
+ to playlist item mapping */
+ void *id_tree; /**< Search tree for item ID to item mapping */
vlc_sd_internal_t **pp_sds;
int i_sds; /**< Number of service discovery modules */
@@ -129,7 +131,7 @@ int playlist_NodeInsert(playlist_t *, playlist_item_t*, playlist_item_t *,
playlist_item_t *playlist_ItemFindFromInputAndRoot( playlist_t *p_playlist,
input_item_t *p_input, playlist_item_t *p_root );
-void playlist_ItemRelease( playlist_item_t * );
+void playlist_ItemRelease( playlist_t *, playlist_item_t * );
void ResetCurrentlyPlaying( playlist_t *p_playlist, playlist_item_t *p_cur );
void ResyncCurrentIndex( playlist_t *p_playlist, playlist_item_t *p_cur );
diff --git a/src/playlist/search.c b/src/playlist/search.c
index 0853b92..ee09ca8 100644
--- a/src/playlist/search.c
+++ b/src/playlist/search.c
@@ -34,52 +34,6 @@
* Item search functions
***************************************************************************/
-/**
- * Search a playlist item by its playlist_item id.
- * The playlist have to be locked
- * @param p_playlist: the playlist
- * @param i_id: the id to find
- * @return the item or NULL on failure
- */
-playlist_item_t* playlist_ItemGetById( playlist_t * p_playlist , int i_id )
-{
- int i;
- PL_ASSERT_LOCKED;
- ARRAY_BSEARCH( pl_priv(p_playlist)->all_items,->i_id, int, i_id, i );
- if( i != -1 )
- return ARRAY_VAL( pl_priv(p_playlist)->all_items, i );
- else
- return NULL;
-}
-
-/**
- * Search an item by its input_item_t
- * The playlist have to be locked
- * @param p_playlist: the playlist
- * @param p_item: the input_item_t to find
- * @return the item, or NULL on failure
- */
-playlist_item_t* playlist_ItemGetByInput( playlist_t * p_playlist,
- const input_item_t *p_item )
-{
- PL_ASSERT_LOCKED;
- if( get_current_status_item( p_playlist ) &&
- get_current_status_item( p_playlist )->p_input == p_item )
- {
- return get_current_status_item( p_playlist );
- }
- /** \todo Check if this is always incremental and whether we can bsearch */
- for( int i = 0 ; i < pl_priv(p_playlist)->all_items.i_size; i++ )
- {
- if( ARRAY_VAL(pl_priv(p_playlist)->all_items, i)->p_input == p_item )
- {
- return ARRAY_VAL(pl_priv(p_playlist)->all_items, i);
- }
- }
- return NULL;
-}
-
-
/***************************************************************************
* Live search handling
***************************************************************************/
diff --git a/src/playlist/tree.c b/src/playlist/tree.c
index b6bc9c3..4ff5a7d 100644
--- a/src/playlist/tree.c
+++ b/src/playlist/tree.c
@@ -74,8 +74,6 @@ playlist_item_t * playlist_NodeCreate( playlist_t *p_playlist,
if( p_item == NULL ) return NULL;
p_item->i_children = 0;
- ARRAY_APPEND(pl_priv(p_playlist)->all_items, p_item);
-
if( p_parent != NULL )
playlist_NodeInsert( p_playlist, p_item, p_parent, i_pos );
playlist_SendAddNotify( p_playlist, p_item,
@@ -109,9 +107,6 @@ void playlist_NodeDelete( playlist_t *p_playlist, playlist_item_t *p_root,
int i;
var_SetAddress( p_playlist, "playlist-item-deleted", p_root );
- ARRAY_BSEARCH( pl_priv(p_playlist)->all_items, ->i_id, int, p_root->i_id, i );
- if( i != -1 )
- ARRAY_REMOVE( pl_priv(p_playlist)->all_items, i );
if( p_root->i_children == -1 ) {
ARRAY_BSEARCH( p_playlist->items,->i_id, int, p_root->i_id, i );
@@ -151,7 +146,7 @@ void playlist_NodeDelete( playlist_t *p_playlist, playlist_item_t *p_root,
}
}
- playlist_ItemRelease( p_root );
+ playlist_ItemRelease( p_playlist, p_root );
}
int playlist_NodeInsert( playlist_t *p_playlist,
More information about the vlc-commits
mailing list