From ddc39977bc530954a2497c96cb78e7def0747908 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Mon, 29 Sep 2008 03:35:54 -0700 Subject: directory: streamline deletes Instead of relying on the shortname, just pass the song pointer to prevent redundant lookups during deletes. --- src/directory.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) (limited to 'src/directory.c') diff --git a/src/directory.c b/src/directory.c index fc9c6b41d..0c34e2ea9 100644 --- a/src/directory.c +++ b/src/directory.c @@ -70,8 +70,7 @@ static enum update_return updateDirectory(Directory * directory); static void deleteEmptyDirectoriesInDirectory(Directory * directory); -static void removeSongFromDirectory(Directory * directory, - const char *shortname); +static void delete_song(Directory *dir, Song *del); static enum update_return addSubDirectoryToDirectory(Directory * directory, const char *name, struct stat *st); @@ -212,16 +211,12 @@ static void freeDirectory(Directory * directory) /*getDirectoryPath(NULL); */ } -static void removeSongFromDirectory(Directory * directory, const char *shortname) +static void delete_song(Directory *dir, Song *del) { - Song *song = songvec_find(&directory->songs, shortname); - - if (song) { - char path_max_tmp[MPD_PATH_MAX]; /* wasteful */ - LOG("removing: %s\n", get_song_url(path_max_tmp, song)); - songvec_delete(&directory->songs, song); - freeSong(song); /* FIXME racy */ - } + char path_max_tmp[MPD_PATH_MAX]; /* wasteful */ + LOG("removing: %s\n", get_song_url(path_max_tmp, del)); + songvec_delete(&dir->songs, del); + freeSong(del); /* FIXME racy */ } static void deleteEmptyDirectoriesInDirectory(Directory * directory) @@ -256,7 +251,7 @@ updateInDirectory(Directory * directory, const char *name) } else if (st.st_mtime != song->mtime) { LOG("updating %s\n", name); if (updateSongInfo(song) < 0) - removeSongFromDirectory(directory, shortname); + delete_song(directory, song); return UPDATE_RETURN_UPDATED; } } else if (S_ISDIR(st.st_mode)) { @@ -308,7 +303,7 @@ removeDeletedFromDirectory(char *path_max_tmp, Directory * directory) strcpy(path_max_tmp, song->url); if (!isFile(path_max_tmp, NULL)) { - removeSongFromDirectory(directory, song->url); + delete_song(directory, song); ret = UPDATE_RETURN_UPDATED; } } @@ -322,6 +317,7 @@ static Directory *addDirectoryPathToDB(const char *utf8path) char *parent; Directory *parentDirectory; Directory *directory; + Song *conflicting; parent = parent_path(path_max_tmp, utf8path); @@ -348,7 +344,10 @@ static Directory *addDirectoryPathToDB(const char *utf8path) /* if we're adding directory paths, make sure to delete filenames with potentially the same name */ - removeSongFromDirectory(parentDirectory, mpd_basename(directory->path)); + conflicting = songvec_find(&parentDirectory->songs, + mpd_basename(directory->path)); + if (conflicting) + delete_song(parentDirectory, conflicting); return directory; } @@ -419,14 +418,13 @@ static enum update_return updatePath(const char *utf8path) else if (updateSongInfo(song) == 0) return UPDATE_RETURN_UPDATED; else { - removeSongFromDirectory(parentDirectory, - song->url); + delete_song(parentDirectory, song); return UPDATE_RETURN_UPDATED; } } /* if updateDirectory fails, means we should delete it */ else { - removeSongFromDirectory(parentDirectory, song->url); + delete_song(parentDirectory, song); ret = UPDATE_RETURN_UPDATED; /* don't return, path maybe a directory now */ } -- cgit v1.2.3 From 8d9a77bfcb2e37728f1a683a74d266d145aff14c Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Fri, 3 Oct 2008 17:03:53 -0700 Subject: directory: simplify list update handling logic Now the "update" command can be issued multiple times regardless of whether the client is in list mode or not. We serialize the update tasks to prevent updates from trampling over each other and will spawn another update task once the current one is finished updating and reaped. Right now we cap the queue size to 32 which is probably enough (I bet most people usually run update with no argument anyways); but we can make it grow/shrink dynamically if needed. There'll still be a hard-coded limit to prevent DoS attacks, though. --- src/directory.c | 120 ++++++++++++++++++++++++++++---------------------------- 1 file changed, 61 insertions(+), 59 deletions(-) (limited to 'src/directory.c') diff --git a/src/directory.c b/src/directory.c index 0c34e2ea9..1cd9da9eb 100644 --- a/src/directory.c +++ b/src/directory.c @@ -52,13 +52,18 @@ enum update_progress { UPDATE_PROGRESS_DONE = 2 } progress; +/* make this dynamic?, or maybe this is big enough... */ +static char *update_paths[32]; +static size_t update_paths_nr; + static Directory *music_root; static time_t directory_dbModTime; static pthread_t update_thr; -static int directory_updateJobId; +static const int update_task_id_max = 1 << 15; +static int update_task_id; static int addToDirectory(Directory * directory, const char *name); @@ -97,87 +102,84 @@ static char *getDbFile(void) int isUpdatingDB(void) { - return (progress != UPDATE_PROGRESS_IDLE) ? directory_updateJobId : 0; -} - -void reap_update_task(void) -{ - enum update_return ret; - - if (progress != UPDATE_PROGRESS_DONE) - return; - if (pthread_join(update_thr, (void **)&ret)) - FATAL("error joining update thread: %s\n", strerror(errno)); - if (ret == UPDATE_RETURN_UPDATED) - playlistVersionChange(); - progress = UPDATE_PROGRESS_IDLE; + return (progress != UPDATE_PROGRESS_IDLE) ? update_task_id : 0; } -/* @argv represents a null-terminated array of (null-terminated) strings */ -static void * update_task(void *argv) +static void * update_task(void *_path) { enum update_return ret = UPDATE_RETURN_NOUPDATE; - if (argv) { - char **pathv; - for (pathv = (char **)argv; *pathv; pathv++) { - switch (updatePath(*pathv)) { - case UPDATE_RETURN_ERROR: - ret = UPDATE_RETURN_ERROR; - goto out; - case UPDATE_RETURN_NOUPDATE: - break; - case UPDATE_RETURN_UPDATED: - ret = UPDATE_RETURN_UPDATED; - } - free(*pathv); - } - free(argv); + if (_path) { + ret = updatePath((char *)_path); + free(_path); } else { ret = updateDirectory(music_root); } if (ret == UPDATE_RETURN_UPDATED && writeDirectoryDB() < 0) ret = UPDATE_RETURN_ERROR; -out: progress = UPDATE_PROGRESS_DONE; wakeup_main_task(); return (void *)ret; } -int updateInit(int argc, char *argv[]) +static void spawn_update_task(char *path) { pthread_attr_t attr; - char **pathv = NULL; - int i; - - if (progress != UPDATE_PROGRESS_IDLE) { - for (i = argc; --i >= 0; ) - free(argv[i]); - return -1; - } - for (i = argc; --i >= 0; ) { - if (!argv[i]) - return -2; - } + assert(pthread_equal(pthread_self(), main_task)); progress = UPDATE_PROGRESS_RUNNING; - if (argc > 0) { - pathv = xmalloc((argc + 1) * sizeof(char *)); - memcpy(pathv, argv, argc * sizeof(char *)); - pathv[argc] = NULL; - } - pthread_attr_init(&attr); - if (pthread_create(&update_thr, &attr, update_task, pathv)) + if (pthread_create(&update_thr, &attr, update_task, path)) FATAL("Failed to spawn update task: %s\n", strerror(errno)); - directory_updateJobId++; - if (directory_updateJobId > 1 << 15) - directory_updateJobId = 1; - DEBUG("updateInit: spawned update thread for update job id %i\n", - (int)directory_updateJobId); - return directory_updateJobId; + if (++update_task_id > update_task_id_max) + update_task_id = 1; + DEBUG("spawned thread for update job id %i\n", update_task_id); +} + +void reap_update_task(void) +{ + enum update_return ret; + + assert(pthread_equal(pthread_self(), main_task)); + + if (progress != UPDATE_PROGRESS_DONE) + return; + if (pthread_join(update_thr, (void **)&ret)) + FATAL("error joining update thread: %s\n", strerror(errno)); + if (ret == UPDATE_RETURN_UPDATED) + playlistVersionChange(); + + if (update_paths_nr) { + char *path = update_paths[0]; + memmove(&update_paths[0], &update_paths[1], + --update_paths_nr * sizeof(char *)); + spawn_update_task(path); + } else { + progress = UPDATE_PROGRESS_IDLE; + } +} + +int directory_update_init(char *path) +{ + assert(pthread_equal(pthread_self(), main_task)); + + if (progress != UPDATE_PROGRESS_IDLE) { + int next_task_id; + + if (!path) + return -1; + if (update_paths_nr == ARRAY_SIZE(update_paths)) + return -1; + assert(update_paths_nr < ARRAY_SIZE(update_paths)); + update_paths[update_paths_nr++] = path; + next_task_id = update_task_id + update_paths_nr; + + return next_task_id > update_task_id_max ? 1 : next_task_id; + } + spawn_update_task(path); + return update_task_id; } static void directory_set_stat(Directory * dir, const struct stat *st) -- cgit v1.2.3