[PATCH 1/4] fix: put tracks with blank artist on top in view 2

[Johannes Weißl]

strcoll() seems to ignore non-alphanumeric characters at the beginning
of a string. The comments_get_albumartist() function returns
"<No Name>" if no artist is defined, which doesn't make sense for
view 2, because tracks end up around artists starting with "No...".

Reported by Jason Woofenden <***@jasonwoof.com>.
---
comment.c | 14 --------------
comment.h | 1 -
tree.c | 28 ++++++++++++++++++++++++++--
3 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/comment.c b/comment.c
index a2bc4bf..198976d 100644
--- a/comment.c
+++ b/comment.c
@@ -41,26 +41,12 @@ int track_is_va_compilation(const struct keyval *comments)
return c && is_freeform_true(c);
}

-const char *comments_get_album(const struct keyval *comments)
-{
- const char *val = keyvals_get_val(comments, "album");
-
- if (!val || strcmp(val, "") == 0)
- val = "<No Name>";
-
- return val;
-}
-
const char *comments_get_albumartist(const struct keyval *comments)
{
const char *val = keyvals_get_val(comments, "albumartist");

- if (track_is_va_compilation(comments))
- val = "<Various Artists>";
if (!val || strcmp(val, "") == 0)
val = keyvals_get_val(comments, "artist");
- if (!val || strcmp(val, "") == 0)
- val = "<No Name>";

return val;
}
diff --git a/comment.h b/comment.h
index 0a27449..635b7ee 100644
--- a/comment.h
+++ b/comment.h
@@ -6,7 +6,6 @@
int track_is_compilation(const struct keyval *comments);
int track_is_va_compilation(const struct keyval *comments);

-const char *comments_get_album(const struct keyval *comments);
const char *comments_get_albumartist(const struct keyval *comments);
const char *comments_get_artistsort(const struct keyval *comments); /* can return NULL */

diff --git a/tree.c b/tree.c
index 617111e..95676bc 100644
--- a/tree.c
+++ b/tree.c
@@ -611,6 +611,30 @@ static void album_add_track(struct album *album, struct tree_track *track)
rb_insert_color(&track->tree_node, &album->track_root);
}

+static const char *tree_artist_name(const struct keyval *comments)
+{
+ const char *val = keyvals_get_val(comments, "albumartist");
+
+ if (track_is_va_compilation(comments))
+ val = "<Various Artists>";
+ if (!val || strcmp(val, "") == 0)
+ val = keyvals_get_val(comments, "artist");
+ if (!val || strcmp(val, "") == 0)
+ val = "<No Name>";
+
+ return val;
+}
+
+static const char *tree_album_name(const struct keyval *comments)
+{
+ const char *val = keyvals_get_val(comments, "album");
+
+ if (!val || strcmp(val, "") == 0)
+ val = "<No Name>";
+
+ return val;
+}
+
void tree_add_track(struct tree_track *track)
{
const struct track_info *ti = tree_track_info(track);
@@ -626,8 +650,8 @@ void tree_add_track(struct tree_track *track)
artist_name = "<Stream>";
album_name = "<Stream>";
} else {
- album_name = comments_get_album(ti->comments);
- artist_name = comments_get_albumartist(ti->comments);
+ album_name = tree_album_name(ti->comments);
+ artist_name = tree_artist_name(ti->comments);
artistsort_name = comments_get_artistsort(ti->comments);

is_va_compilation = track_is_va_compilation(ti->comments);

[Johannes Weißl]

* represent often used comments as members in track_info
* replace string sort keys array with integer array

speedup: 21% cpu time for initial indexing (~30k files)
---
cache.c | 6 ++--
editable.c | 31 +++-----------------
editable.h | 4 +-
options.c | 89 +++++++++++++++++++++++++++++++++++++++++-----------------
player.c | 2 +-
track.c | 3 +-
track.h | 3 +-
track_info.c | 85 ++++++++++++++++++++++++++-----------------------------
track_info.h | 32 ++++++++++++++++++++-
tree.c | 26 ++++++++---------
ui_curses.c | 21 ++++++--------
utils.h | 2 +
12 files changed, 171 insertions(+), 133 deletions(-)

diff --git a/cache.c b/cache.c
index ffe5f1c..f999036 100644
--- a/cache.c
+++ b/cache.c
@@ -112,8 +112,7 @@ static struct track_info *cache_entry_to_ti(struct cache_entry *e)

// NOTE: filename already copied by track_info_new()
pos = strlen(strings) + 1;
- ti->comments = xnew(struct keyval, count + 1);
- kv = ti->comments;
+ kv = xnew(struct keyval, count + 1);
for (i = 0; i < count; i++) {
int size;

@@ -127,6 +126,7 @@ static struct track_info *cache_entry_to_ti(struct cache_entry *e)
}
kv[i].key = NULL;
kv[i].val = NULL;
+ track_info_set_comments(ti, kv);
return ti;
}

@@ -365,7 +365,7 @@ static struct track_info *ip_get_ti(const char *filename)
rc = ip_read_comments(ip, &comments);
if (!rc) {
ti = track_info_new(filename);
- ti->comments = comments;
+ track_info_set_comments(ti, comments);
ti->duration = ip_duration(ip);
ti->mtime = ip_is_remote(ip) ? -1 : file_get_mtime(filename);
}
diff --git a/editable.c b/editable.c
index 9ff5e6d..e09f8ff 100644
--- a/editable.c
+++ b/editable.c
@@ -39,8 +39,8 @@ void editable_init(struct editable *e, void (*free_track)(struct list_head *item
e->nr_tracks = 0;
e->nr_marked = 0;
e->total_time = 0;
- e->sort_keys = xnew(const char *, 1);
- e->sort_keys[0] = NULL;
+ e->sort_keys = xnew(sort_key_t, 1);
+ e->sort_keys[0] = SORT_INVALID;
e->sort_str[0] = 0;
e->free_track = free_track;

@@ -121,32 +121,11 @@ void editable_sort(struct editable *e)
window_goto_top(e->win);
}

-static void keys_to_str(const char **keys, char *buf, size_t bufsize)
-{
- int i, pos = 0;
-
- for (i = 0; keys[i]; i++) {
- const char *key = keys[i];
- int len = strlen(key);
-
- if ((int)bufsize - pos - len - 2 < 0)
- break;
-
- memcpy(buf + pos, key, len);
- pos += len;
- buf[pos++] = ' ';
- }
- if (pos > 0)
- pos--;
- buf[pos] = 0;
-}
-
-void editable_set_sort_keys(struct editable *e, const char **keys)
+void editable_set_sort_keys(struct editable *e, sort_key_t *keys)
{
free(e->sort_keys);
e->sort_keys = keys;
editable_sort(e);
- keys_to_str(keys, e->sort_str, sizeof(e->sort_str));
}

void editable_toggle_mark(struct editable *e)
@@ -261,7 +240,7 @@ void editable_move_after(struct editable *e)
{
struct simple_track *sel;

- if (e->nr_tracks <= 1 || e->sort_keys[0])
+ if (e->nr_tracks <= 1 || e->sort_keys[0] != SORT_INVALID)
return;

sel = get_selected(e);
@@ -273,7 +252,7 @@ void editable_move_before(struct editable *e)
{
struct simple_track *sel;

- if (e->nr_tracks <= 1 || e->sort_keys[0])
+ if (e->nr_tracks <= 1 || e->sort_keys[0] != SORT_INVALID)
return;

sel = get_selected(e);
diff --git a/editable.h b/editable.h
index bff11b7..84e8413 100644
--- a/editable.h
+++ b/editable.h
@@ -18,7 +18,7 @@ struct editable {
unsigned int nr_tracks;
unsigned int nr_marked;
unsigned int total_time;
- const char **sort_keys;
+ sort_key_t *sort_keys;
char sort_str[128];
struct searchable *searchable;

@@ -32,7 +32,7 @@ void editable_add(struct editable *e, struct simple_track *track);
void editable_remove_track(struct editable *e, struct simple_track *track);
void editable_remove_sel(struct editable *e);
void editable_sort(struct editable *e);
-void editable_set_sort_keys(struct editable *e, const char **keys);
+void editable_set_sort_keys(struct editable *e, sort_key_t *keys);
void editable_toggle_mark(struct editable *e);
void editable_move_after(struct editable *e);
void editable_move_before(struct editable *e);
diff --git a/options.c b/options.c
index 0ee8e8d..43ab196 100644
--- a/options.c
+++ b/options.c
@@ -22,6 +22,7 @@
#include "prog.h"
#include "output.h"
#include "config/datadir.h"
+#include "track_info.h"

#include <stdio.h>
#include <errno.h>
@@ -184,30 +185,33 @@ static void set_id3_default_charset(unsigned int id, const char *buf)
id3_default_charset = xstrdup(buf);
}

-static const char * const valid_sort_keys[] = {
- "artist",
- "album",
- "title",
- "tracknumber",
- "discnumber",
- "date",
- "genre",
- "comment",
- "filename",
- "albumartist",
- "filemtime",
- NULL
+static const struct {
+ const char *str;
+ sort_key_t key;
+} sort_key_map[] = {
+ { "artist", SORT_ARTIST },
+ { "album", SORT_ALBUM },
+ { "title", SORT_TITLE },
+ { "tracknumber", SORT_TRACKNUMBER },
+ { "discnumber", SORT_DISCNUMBER },
+ { "date", SORT_DATE },
+ { "genre", SORT_GENRE },
+ { "comment", SORT_COMMENT },
+ { "albumartist", SORT_ALBUMARTIST },
+ { "filename", SORT_FILENAME },
+ { "filemtime", SORT_FILEMTIME },
+ { NULL, SORT_INVALID }
};

-static const char **parse_sort_keys(const char *value)
+static sort_key_t *parse_sort_keys(const char *value)
{
- const char **keys;
+ sort_key_t *keys;
const char *s, *e;
int size = 4;
int pos = 0;

size = 4;
- keys = xnew(const char *, size);
+ keys = xnew(sort_key_t, size);

s = value;
while (1) {
@@ -232,26 +236,55 @@ static const char **parse_sort_keys(const char *value)
s = e;

for (i = 0; ; i++) {
- if (valid_sort_keys[i] == NULL) {
+ if (sort_key_map[i].str == NULL) {
error_msg("invalid sort key '%s'", buf);
free(keys);
return NULL;
}

- if (strcmp(buf, valid_sort_keys[i]) == 0)
+ if (strcmp(buf, sort_key_map[i].str) == 0)
break;
}
-
if (pos == size - 1) {
size *= 2;
- keys = xrenew(const char *, keys, size);
+ keys = xrenew(sort_key_t, keys, size);
}
- keys[pos++] = valid_sort_keys[i];
+ keys[pos++] = sort_key_map[i].key;
}
- keys[pos] = NULL;
+ keys[pos] = SORT_INVALID;
return keys;
}

+static const char *sort_key_to_str(sort_key_t key)
+{
+ int i;
+ for (i = 0; sort_key_map[i].str; i++) {
+ if (sort_key_map[i].key == key)
+ return sort_key_map[i].str;
+ }
+ return NULL;
+}
+
+static void sort_keys_to_str(const sort_key_t *keys, char *buf, size_t bufsize)
+{
+ int i, pos = 0;
+
+ for (i = 0; keys[i] != SORT_INVALID; i++) {
+ const char *key = sort_key_to_str(keys[i]);
+ int len = strlen(key);
+
+ if ((int)bufsize - pos - len - 2 < 0)
+ break;
+
+ memcpy(buf + pos, key, len);
+ pos += len;
+ buf[pos++] = ' ';
+ }
+ if (pos > 0)
+ pos--;
+ buf[pos] = 0;
+}
+
static void get_lib_sort(unsigned int id, char *buf)
{
strcpy(buf, lib_editable.sort_str);
@@ -259,10 +292,12 @@ static void get_lib_sort(unsigned int id, char *buf)

static void set_lib_sort(unsigned int id, const char *buf)
{
- const char **keys = parse_sort_keys(buf);
+ sort_key_t *keys = parse_sort_keys(buf);

- if (keys)
+ if (keys) {
editable_set_sort_keys(&lib_editable, keys);
+ sort_keys_to_str(keys, lib_editable.sort_str, sizeof(lib_editable.sort_str));
+ }
}

static void get_pl_sort(unsigned int id, char *buf)
@@ -272,10 +307,12 @@ static void get_pl_sort(unsigned int id, char *buf)

static void set_pl_sort(unsigned int id, const char *buf)
{
- const char **keys = parse_sort_keys(buf);
+ sort_key_t *keys = parse_sort_keys(buf);

- if (keys)
+ if (keys) {
editable_set_sort_keys(&pl_editable, keys);
+ sort_keys_to_str(keys, pl_editable.sort_str, sizeof(pl_editable.sort_str));
+ }
}

static void get_output_plugin(unsigned int id, char *buf)
diff --git a/player.c b/player.c
index fc1ec4f..9b97969 100644
--- a/player.c
+++ b/player.c
@@ -355,7 +355,7 @@ static inline void metadata_changed(void)
if (!rc) {
if (player_info.ti->comments)
keyvals_free(player_info.ti->comments);
- player_info.ti->comments = comments;
+ track_info_set_comments(player_info.ti, comments);
}

player_info.metadata_changed = 1;
diff --git a/track.c b/track.c
index ed199d8..8c63319 100644
--- a/track.c
+++ b/track.c
@@ -4,7 +4,6 @@

#include "track.h"
#include "iter.h"
-#include "track_info.h"
#include "search_mode.h"
#include "window.h"
#include "options.h"
@@ -151,7 +150,7 @@ again:
return NULL;
}

-void sorted_list_add_track(struct list_head *head, struct rb_root *tree_root, struct simple_track *track, const char * const *keys)
+void sorted_list_add_track(struct list_head *head, struct rb_root *tree_root, struct simple_track *track, const sort_key_t *keys)
{
struct rb_node **new = &(tree_root->rb_node), *parent = NULL, *curr, *prev, *next;

diff --git a/track.h b/track.h
index 6735a53..e6eeec9 100644
--- a/track.h
+++ b/track.h
@@ -8,6 +8,7 @@
#include "list.h"
#include "rbtree.h"
#include "iter.h"
+#include "track_info.h"

struct simple_track {
struct list_head node;
@@ -72,7 +73,7 @@ struct simple_track *simple_list_get_next(struct list_head *head, struct simple_
struct simple_track *simple_list_get_prev(struct list_head *head, struct simple_track *cur,
int (*filter)(const struct simple_track *));

-void sorted_list_add_track(struct list_head *head, struct rb_root *tree_root, struct simple_track *track, const char * const *keys);
+void sorted_list_add_track(struct list_head *head, struct rb_root *tree_root, struct simple_track *track, const sort_key_t *keys);

void list_add_rand(struct list_head *head, struct list_head *node, int nr);

diff --git a/track_info.c b/track_info.c
index 334ac9e..b211faf 100644
--- a/track_info.c
+++ b/track_info.c
@@ -41,9 +41,25 @@ struct track_info *track_info_new(const char *filename)
ti = xnew(struct track_info, 1);
ti->filename = xstrdup(filename);
ti->ref = 1;
+ ti->comments = NULL;
return ti;
}

+void track_info_set_comments(struct track_info *ti, struct keyval *comments) {
+ ti->comments = comments;
+ ti->artist = keyvals_get_val(comments, "artist");
+ ti->album = keyvals_get_val(comments, "album");
+ ti->title = keyvals_get_val(comments, "title");
+ ti->tracknumber = comments_get_int(comments, "tracknumber");
+ ti->discnumber = comments_get_int(comments, "discnumber");
+ ti->date = comments_get_date(comments, "date");
+ ti->genre = keyvals_get_val(comments, "genre");
+ ti->comment = keyvals_get_val(comments, "comment");
+ ti->albumartist = comments_get_albumartist(comments);
+ ti->artistsort = comments_get_artistsort(comments);
+ ti->is_va_compilation = track_is_va_compilation(comments);
+}
+
void track_info_ref(struct track_info *ti)
{
BUG_ON(ti->ref < 1);
@@ -60,17 +76,15 @@ void track_info_unref(struct track_info *ti)

int track_info_has_tag(const struct track_info *ti)
{
- return keyvals_get_val(ti->comments, "artist") ||
- keyvals_get_val(ti->comments, "album") ||
- keyvals_get_val(ti->comments, "title");
+ return ti->artist || ti->album || ti->title;
}

int track_info_matches(struct track_info *ti, const char *text, unsigned int flags)
{
- const char *artist = keyvals_get_val(ti->comments, "artist");
- const char *album = keyvals_get_val(ti->comments, "album");
- const char *title = keyvals_get_val(ti->comments, "title");
- const char *albumartist = keyvals_get_val(ti->comments, "albumartist");
+ const char *artist = ti->artist;
+ const char *album = ti->album;
+ const char *title = ti->title;
+ const char *albumartist = ti->albumartist;
char **words;
int i, matched = 1;

@@ -111,54 +125,35 @@ int track_info_matches(struct track_info *ti, const char *text, unsigned int fla
return matched;
}

-int track_info_cmp(const struct track_info *a, const struct track_info *b, const char * const *keys)
+/* this function gets called *alot*, it must be very fast */
+int track_info_cmp(const struct track_info *a, const struct track_info *b, const sort_key_t *keys)
{
int i, res = 0;

- for (i = 0; keys[i]; i++) {
- const char *key = keys[i];
+ for (i = 0; keys[i] != SORT_INVALID; i++) {
+ sort_key_t key = keys[i];
const char *av, *bv;

- /* numeric compare for tracknumber and discnumber */
- if (strcmp(key, "tracknumber") == 0) {
- res = comments_get_int(a->comments, key) -
- comments_get_int(b->comments, key);
- if (res)
- break;
- continue;
- }
- if (strcmp(key, "discnumber") == 0) {
- res = comments_get_int(a->comments, key) -
- comments_get_int(b->comments, key);
- if (res)
- break;
- continue;
- }
- if (strcmp(key, "filename") == 0) {
+ switch (key) {
+ case SORT_TRACKNUMBER:
+ case SORT_DISCNUMBER:
+ case SORT_DATE:
+ res = getentry(a, key, int) - getentry(b, key, int);
+ break;
+ case SORT_FILEMTIME:
+ res = a->mtime - b->mtime;
+ break;
+ case SORT_FILENAME:
/* NOTE: filenames are not necessarily UTF-8 */
res = strcoll(a->filename, b->filename);
- if (res)
- break;
- continue;
- }
- if (strcmp(key, "albumartist") == 0) {
- av = comments_get_albumartist(a->comments);
- bv = comments_get_albumartist(b->comments);
+ break;
+ default:
+ av = getentry(a, key, const char *);
+ bv = getentry(b, key, const char *);
res = u_strcasecoll0(av, bv);
- if (res)
- break;
- continue;
- }
- if (strcmp(key, "filemtime") == 0) {
- res = a->mtime - b->mtime;
- if (res)
- break;
- continue;
+ break;
}

- av = keyvals_get_val(a->comments, key);
- bv = keyvals_get_val(b->comments, key);
- res = u_strcasecoll0(av, bv);
if (res)
break;
}
diff --git a/track_info.h b/track_info.h
index 8353940..586948a 100644
--- a/track_info.h
+++ b/track_info.h
@@ -21,6 +21,7 @@
#define _TRACK_INFO_H

#include <time.h>
+#include <stddef.h>

struct track_info {
struct keyval *comments;
@@ -32,8 +33,36 @@ struct track_info {
int duration;
int ref;
char *filename;
+
+ int tracknumber;
+ int discnumber;
+ int date;
+ const char *artist;
+ const char *album;
+ const char *title;
+ const char *genre;
+ const char *comment;
+ const char *albumartist;
+ const char *artistsort;
+
+ int is_va_compilation : 1;
};

+typedef size_t sort_key_t;
+
+#define SORT_ARTIST offsetof(struct track_info, artist)
+#define SORT_ALBUM offsetof(struct track_info, album)
+#define SORT_TITLE offsetof(struct track_info, title)
+#define SORT_TRACKNUMBER offsetof(struct track_info, tracknumber)
+#define SORT_DISCNUMBER offsetof(struct track_info, discnumber)
+#define SORT_DATE offsetof(struct track_info, date)
+#define SORT_GENRE offsetof(struct track_info, genre)
+#define SORT_COMMENT offsetof(struct track_info, comment)
+#define SORT_ALBUMARTIST offsetof(struct track_info, albumartist)
+#define SORT_FILENAME offsetof(struct track_info, filename)
+#define SORT_FILEMTIME offsetof(struct track_info, mtime)
+#define SORT_INVALID ((sort_key_t) (-1))
+
#define TI_MATCH_ARTIST (1 << 0)
#define TI_MATCH_ALBUM (1 << 1)
#define TI_MATCH_TITLE (1 << 2)
@@ -41,6 +70,7 @@ struct track_info {

/* initializes only filename and ref */
struct track_info *track_info_new(const char *filename);
+void track_info_set_comments(struct track_info *ti, struct keyval *comments);

void track_info_ref(struct track_info *ti);
void track_info_unref(struct track_info *ti);
@@ -59,6 +89,6 @@ int track_info_has_tag(const struct track_info *ti);
*/
int track_info_matches(struct track_info *ti, const char *text, unsigned int flags);

-int track_info_cmp(const struct track_info *a, const struct track_info *b, const char * const *keys);
+int track_info_cmp(const struct track_info *a, const struct track_info *b, const sort_key_t *keys);

#endif
diff --git a/tree.c b/tree.c
index 95676bc..a4de3ee 100644
--- a/tree.c
+++ b/tree.c
@@ -586,8 +586,8 @@ static void album_add_track(struct album *album, struct tree_track *track)
* have all track numbers set or all unset (within one album
* of course).
*/
- static const char * const album_track_sort_keys[] = {
- "discnumber", "tracknumber", "filename", NULL
+ static const sort_key_t album_track_sort_keys[] = {
+ SORT_DISCNUMBER, SORT_TRACKNUMBER, SORT_FILENAME, SORT_INVALID
};
struct rb_node **new = &(album->track_root.rb_node), *parent = NULL;

@@ -611,23 +611,21 @@ static void album_add_track(struct album *album, struct tree_track *track)
rb_insert_color(&track->tree_node, &album->track_root);
}

-static const char *tree_artist_name(const struct keyval *comments)
+static const char *tree_artist_name(const struct track_info* ti)
{
- const char *val = keyvals_get_val(comments, "albumartist");
+ const char *val = ti->albumartist;

- if (track_is_va_compilation(comments))
+ if (ti->is_va_compilation)
val = "<Various Artists>";
if (!val || strcmp(val, "") == 0)
- val = keyvals_get_val(comments, "artist");
- if (!val || strcmp(val, "") == 0)
val = "<No Name>";

return val;
}

-static const char *tree_album_name(const struct keyval *comments)
+static const char *tree_album_name(const struct track_info* ti)
{
- const char *val = keyvals_get_val(comments, "album");
+ const char *val = ti->album;

if (!val || strcmp(val, "") == 0)
val = "<No Name>";
@@ -644,17 +642,17 @@ void tree_add_track(struct tree_track *track)
int date;
int is_va_compilation = 0;

- date = comments_get_date(ti->comments, "date");
+ date = ti->date;

if (is_url(ti->filename)) {
artist_name = "<Stream>";
album_name = "<Stream>";
} else {
- album_name = tree_album_name(ti->comments);
- artist_name = tree_artist_name(ti->comments);
- artistsort_name = comments_get_artistsort(ti->comments);
+ album_name = tree_album_name(ti);
+ artist_name = tree_artist_name(ti);
+ artistsort_name = ti->artistsort;

- is_va_compilation = track_is_va_compilation(ti->comments);
+ is_va_compilation = ti->is_va_compilation;
}

new_artist = artist_new(artist_name, artistsort_name);
diff --git a/ui_curses.c b/ui_curses.c
index d217d7d..e986a60 100644
--- a/ui_curses.c
+++ b/ui_curses.c
@@ -490,7 +490,6 @@ static inline void fopt_set_time(struct format_option *fopt, int value, int empt
static void fill_track_fopts_track_info(struct track_info *info)
{
char *filename;
- int num, disc;

if (using_utf8) {
filename = info->filename;
@@ -498,18 +497,16 @@ static void fill_track_fopts_track_info(struct track_info *info)
utf8_encode(info->filename);
filename = conv_buffer;
}
- disc = comments_get_int(info->comments, "discnumber");
- num = comments_get_int(info->comments, "tracknumber");

- fopt_set_str(&track_fopts[TF_ALBUMARTIST], comments_get_albumartist(info->comments));
- fopt_set_str(&track_fopts[TF_ARTIST], keyvals_get_val(info->comments, "artist"));
- fopt_set_str(&track_fopts[TF_ALBUM], keyvals_get_val(info->comments, "album"));
- fopt_set_int(&track_fopts[TF_DISC], disc, disc == -1);
- fopt_set_int(&track_fopts[TF_TRACK], num, num == -1);
- fopt_set_str(&track_fopts[TF_TITLE], keyvals_get_val(info->comments, "title"));
+ fopt_set_str(&track_fopts[TF_ALBUMARTIST], info->albumartist);
+ fopt_set_str(&track_fopts[TF_ARTIST], info->artist);
+ fopt_set_str(&track_fopts[TF_ALBUM], info->album);
+ fopt_set_int(&track_fopts[TF_DISC], info->discnumber, info->discnumber == -1);
+ fopt_set_int(&track_fopts[TF_TRACK], info->tracknumber, info->tracknumber == -1);
+ fopt_set_str(&track_fopts[TF_TITLE], info->title);
fopt_set_str(&track_fopts[TF_YEAR], keyvals_get_val(info->comments, "date"));
- fopt_set_str(&track_fopts[TF_GENRE], keyvals_get_val(info->comments, "genre"));
- fopt_set_str(&track_fopts[TF_COMMENT], keyvals_get_val(info->comments, "comment"));
+ fopt_set_str(&track_fopts[TF_GENRE], info->genre);
+ fopt_set_str(&track_fopts[TF_COMMENT], info->comment);
fopt_set_time(&track_fopts[TF_DURATION], info->duration, info->duration == -1);
fopt_set_str(&track_fopts[TF_PATHFILE], filename);
if (is_url(info->filename)) {
diff --git a/utils.h b/utils.h
index 87d5925..4f28ef5 100644
--- a/utils.h
+++ b/utils.h
@@ -30,6 +30,8 @@

#define N_ELEMENTS(array) (sizeof(array) / sizeof((array)[0]))

+#define getentry(ptr, offset, type) (*((type *) ((char *) (ptr) + (offset))))
+
static inline int min(int a, int b)
{
return a < b ? a : b;

[Gregory Petrosyan]

Post by Johannes Weißl
+/* this function gets called *alot*, it must be very fast */

The only issue I've found in this patchset :-)

http://hyperboleandahalf.blogspot.com/2010/04/alot-is-better-than-you-at-everything.html

Thanks Johannes, merged to master!

Gregory

[Johannes Weißl]

Post by Gregory Petrosyan

Post by Johannes Weißl
+/* this function gets called *alot*, it must be very fast */

The only issue I've found in this patchset :-)
http://hyperboleandahalf.blogspot.com/2010/04/alot-is-better-than-you-at-everything.html
Thanks Johannes, merged to master!

Thanks! The alot-pictures are great, I wasn't aware of the error when I
wrote the comment :-).

I've attached a patch with a faster (hybrid) version of u_get_char()
which you wanted me to submit earlier. Now time is right for the
live-filter patches!


Johannes

[Gregory Petrosyan]

Post by Johannes Weißl

Post by Gregory Petrosyan

Post by Johannes Weißl
+/* this function gets called *alot*, it must be very fast */

The only issue I've found in this patchset :-)
http://hyperboleandahalf.blogspot.com/2010/04/alot-is-better-than-you-at-everything.html
Thanks Johannes, merged to master!

Thanks! The alot-pictures are great, I wasn't aware of the error when I
wrote the comment :-).

Yeah, I like these pictures alot ;-)

Post by Johannes Weißl
I've attached a patch with a faster (hybrid) version of u_get_char()
which you wanted me to submit earlier. Now time is right for the
live-filter patches!

New version is much nicer, but it did contain a mistake, which I've fixed as
follows:

- u = ch & first_byte_mask[len];
+ u = ch & first_byte_mask[len - 1];

Thanks for the patch, I've pushed the fixed version to master. Waiting for the
first version of the live-filter patchset now, oh yeah!

Gregory

[Johannes Weißl]

Post by Gregory Petrosyan

Post by Johannes Weißl
I've attached a patch with a faster (hybrid) version of u_get_char()
which you wanted me to submit earlier. Now time is right for the
live-filter patches!

New version is much nicer, but it did contain a mistake, which I've fixed as
- u = ch & first_byte_mask[len];
+ u = ch & first_byte_mask[len - 1];
Thanks for the patch, I've pushed the fixed version to master. Waiting for the
first version of the live-filter patchset now, oh yeah!

Thanks, missed that completely!

Johannes

[Johannes Weißl]

It is much faster to generate collation keys and use normal strcmp()
for comparison.

speedup: 12% cpu time for initial indexing (~30k files)
---
lib.h | 4 ++++
tree.c | 32 ++++++++++++++++++++++++++++----
2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/lib.h b/lib.h
index ef4f610..2de0043 100644
--- a/lib.h
+++ b/lib.h
@@ -42,6 +42,7 @@ struct album {

struct artist *artist;
char *name;
+ char *collkey_name;
/* date of the first track added to this album */
int date;

@@ -58,6 +59,9 @@ struct artist {
char *name;
char *sort_name;
char *auto_sort_name;
+ char *collkey_name;
+ char *collkey_sort_name;
+ char *collkey_auto_sort_name;

/* albums visible for this artist in the tree_win? */
unsigned int expanded : 1;
diff --git a/tree.c b/tree.c
index a4de3ee..0871b5d 100644
--- a/tree.c
+++ b/tree.c
@@ -358,6 +358,9 @@ static struct artist *artist_new(const char *name, const char *sort_name)
a->name = xstrdup(name);
a->sort_name = sort_name ? xstrdup(sort_name) : NULL;
a->auto_sort_name = auto_artist_sort_name(name);
+ a->collkey_name = u_strcasecoll_key(a->name);
+ a->collkey_sort_name = u_strcasecoll_key0(a->sort_name);
+ a->collkey_auto_sort_name = u_strcasecoll_key0(a->auto_sort_name);
a->expanded = 0;
rb_root_init(&a->album_root);

@@ -369,6 +372,9 @@ static void artist_free(struct artist *artist)
free(artist->name);
free(artist->sort_name);
free(artist->auto_sort_name);
+ free(artist->collkey_name);
+ free(artist->collkey_sort_name);
+ free(artist->collkey_auto_sort_name);
free(artist);
}

@@ -377,6 +383,7 @@ static struct album *album_new(struct artist *artist, const char *name, int date
struct album *album = xnew(struct album, 1);

album->name = xstrdup(name);
+ album->collkey_name = u_strcasecoll_key(name);
album->date = date;
rb_root_init(&album->track_root);
album->artist = artist;
@@ -388,6 +395,7 @@ static struct album *album_new(struct artist *artist, const char *name, int date
static void album_free(struct album *album)
{
free(album->name);
+ free(album->collkey_name);
free(album);
}

@@ -442,14 +450,15 @@ struct track_info *tree_set_selected(void)
return info;
}

-static int special_name_cmp(const char *a, const char *b)
+static int special_name_cmp(const char *a, const char *collkey_a,
+ const char *b, const char *collkey_b)
{
/* keep <Stream> etc. top */
int cmp = (*a != '<') - (*b != '<');

if (cmp)
return cmp;
- return u_strcasecoll(a, b);
+ return strcmp(collkey_a, collkey_b);
}

static int special_album_cmp(const struct album *a, const struct album *b)
@@ -467,7 +476,19 @@ static int special_album_cmp(const struct album *a, const struct album *b)
if (a->date != b->date && !a->is_compilation && !b->is_compilation)
return a->date - b->date;

- return u_strcasecoll(a->name, b->name);
+ return strcmp(a->collkey_name, b->collkey_name);
+}
+
+/* has to follow the same logic as artist_sort_name() */
+static inline const char *artist_sort_collkey(const struct artist *a)
+{
+ if (a->sort_name)
+ return a->collkey_sort_name;
+
+ if (smart_artist_sort && a->auto_sort_name)
+ return a->collkey_auto_sort_name;
+
+ return a->collkey_name;
}

static struct artist *do_find_artist(const struct artist *artist,
@@ -477,11 +498,13 @@ static struct artist *do_find_artist(const struct artist *artist,
{
struct rb_node **new = &(root->rb_node), *parent = NULL;
const char *a = artist_sort_name(artist);
+ const char *collkey_a = artist_sort_collkey(artist);

while (*new) {
struct artist *cur_artist = to_artist(*new);
const char *b = artist_sort_name(cur_artist);
- int result = special_name_cmp(a, b);
+ const char *collkey_b = artist_sort_collkey(cur_artist);
+ int result = special_name_cmp(a, collkey_a, b, collkey_b);

parent = *new;
if (result < 0)
@@ -672,6 +695,7 @@ void tree_add_track(struct tree_track *track)
/* If it makes sense to update sort_name, do it */
if (!artist->sort_name && artistsort_name) {
artist->sort_name = xstrdup(artistsort_name);
+ artist->collkey_sort_name = u_strcasecoll_key(artistsort_name);
window_changed(lib_tree_win);
}
}

[Johannes Weißl]

It is much faster to generate collation keys and use normal strcmp()
for comparison.

speedup: 35% cpu time for initial indexing (~30k files)
---
track_info.c | 15 ++++++++++++-
track_info.h | 19 ++++++++++++-----
u_collate.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
u_collate.h | 27 ++++++++++++++++++++++++
utils.h | 10 +++++++++
5 files changed, 127 insertions(+), 7 deletions(-)

diff --git a/track_info.c b/track_info.c
index b211faf..16a88ed 100644
--- a/track_info.c
+++ b/track_info.c
@@ -32,6 +32,12 @@ static void track_info_free(struct track_info *ti)
{
keyvals_free(ti->comments);
free(ti->filename);
+ free(ti->collkey_artist);
+ free(ti->collkey_album);
+ free(ti->collkey_title);
+ free(ti->collkey_genre);
+ free(ti->collkey_comment);
+ free(ti->collkey_albumartist);
free(ti);
}

@@ -58,6 +64,13 @@ void track_info_set_comments(struct track_info *ti, struct keyval *comments) {
ti->albumartist = comments_get_albumartist(comments);
ti->artistsort = comments_get_artistsort(comments);
ti->is_va_compilation = track_is_va_compilation(comments);
+
+ ti->collkey_artist = u_strcasecoll_key0(ti->artist);
+ ti->collkey_album = u_strcasecoll_key0(ti->album);
+ ti->collkey_title = u_strcasecoll_key0(ti->title);
+ ti->collkey_genre = u_strcasecoll_key0(ti->genre);
+ ti->collkey_comment = u_strcasecoll_key0(ti->comment);
+ ti->collkey_albumartist = u_strcasecoll_key0(ti->albumartist);
}

void track_info_ref(struct track_info *ti)
@@ -150,7 +163,7 @@ int track_info_cmp(const struct track_info *a, const struct track_info *b, const
default:
av = getentry(a, key, const char *);
bv = getentry(b, key, const char *);
- res = u_strcasecoll0(av, bv);
+ res = strcmp0(av, bv);
break;
}

diff --git a/track_info.h b/track_info.h
index 586948a..74708d5 100644
--- a/track_info.h
+++ b/track_info.h
@@ -45,20 +45,27 @@ struct track_info {
const char *albumartist;
const char *artistsort;

+ char *collkey_artist;
+ char *collkey_album;
+ char *collkey_title;
+ char *collkey_genre;
+ char *collkey_comment;
+ char *collkey_albumartist;
+
int is_va_compilation : 1;
};

typedef size_t sort_key_t;

-#define SORT_ARTIST offsetof(struct track_info, artist)
-#define SORT_ALBUM offsetof(struct track_info, album)
-#define SORT_TITLE offsetof(struct track_info, title)
+#define SORT_ARTIST offsetof(struct track_info, collkey_artist)
+#define SORT_ALBUM offsetof(struct track_info, collkey_album)
+#define SORT_TITLE offsetof(struct track_info, collkey_title)
#define SORT_TRACKNUMBER offsetof(struct track_info, tracknumber)
#define SORT_DISCNUMBER offsetof(struct track_info, discnumber)
#define SORT_DATE offsetof(struct track_info, date)
-#define SORT_GENRE offsetof(struct track_info, genre)
-#define SORT_COMMENT offsetof(struct track_info, comment)
-#define SORT_ALBUMARTIST offsetof(struct track_info, albumartist)
+#define SORT_GENRE offsetof(struct track_info, collkey_genre)
+#define SORT_COMMENT offsetof(struct track_info, collkey_comment)
+#define SORT_ALBUMARTIST offsetof(struct track_info, collkey_albumartist)
#define SORT_FILENAME offsetof(struct track_info, filename)
#define SORT_FILEMTIME offsetof(struct track_info, mtime)
#define SORT_INVALID ((sort_key_t) (-1))
diff --git a/u_collate.c b/u_collate.c
index 8dba7af..b895ff1 100644
--- a/u_collate.c
+++ b/u_collate.c
@@ -25,6 +25,7 @@

#include <stdlib.h>
#include <string.h>
+#include <limits.h>

int u_strcoll(const char *str1, const char *str2)
{
@@ -77,3 +78,65 @@ int u_strcasecoll0(const char *str1, const char *str2)

return u_strcasecoll(str1, str2);
}
+
+char *u_strcoll_key(const char *str)
+{
+ char *result = NULL;
+
+ if (using_utf8) {
+ size_t xfrm_len = strxfrm(NULL, str, 0);
+ if ((ssize_t) xfrm_len >= 0 && xfrm_len < INT_MAX - 2) {
+ result = xnew(char, xfrm_len + 1);
+ strxfrm(result, str, xfrm_len + 1);
+ }
+ }
+
+ if (!result) {
+ char *str_locale;
+ size_t xfrm_len;
+
+ convert(str, -1, &str_locale, -1, charset, "UTF-8");
+
+ if (str_locale) {
+ xfrm_len = strxfrm(NULL, str_locale, 0);
+ if ((ssize_t) xfrm_len < 0 || xfrm_len >= INT_MAX - 2) {
+ free(str_locale);
+ str_locale = NULL;
+ }
+ }
+ if (str_locale) {
+ result = xnew(char, xfrm_len + 2);
+ result[0] = 'A';
+ strxfrm(result + 1, str_locale, xfrm_len + 1);
+ free(str_locale);
+ }
+ }
+
+ if (!result) {
+ size_t xfrm_len = strlen(str);
+ result = xmalloc(xfrm_len + 2);
+ result[0] = 'B';
+ memcpy(result + 1, str, xfrm_len);
+ result[xfrm_len+1] = '\0';
+ }
+
+ return result;
+}
+
+char *u_strcasecoll_key(const char *str)
+{
+ char *key, *cf_str;
+
+ cf_str = u_casefold(str);
+
+ key = u_strcoll_key(cf_str);
+
+ free(cf_str);
+
+ return key;
+}
+
+char *u_strcasecoll_key0(const char *str)
+{
+ return str ? u_strcasecoll_key(str) : NULL;
+}
diff --git a/u_collate.h b/u_collate.h
index 576df57..4601662 100644
--- a/u_collate.h
+++ b/u_collate.h
@@ -48,4 +48,31 @@ int u_strcasecoll(const char *str1, const char *str2);
*/
int u_strcasecoll0(const char *str1, const char *str2);

+/*
+ * @str valid, normalized, null-terminated UTF-8 string
+ *
+ * Converts a string into a collation key that can be compared
+ * with other collation keys produced by the same function using
+ * strcmp().
+ *
+ * Returns a newly allocated string.
+ */
+char *u_strcoll_key(const char *str);
+
+/*
+ * @str valid, normalized, null-terminated UTF-8 string
+ *
+ * Like u_strcoll_key(), but do casefolding before generating key.
+ *
+ * Returns a newly allocated string.
+ */
+char *u_strcasecoll_key(const char *str);
+
+/*
+ * @str valid, normalized, null-terminated UTF-8 string or NULL
+ *
+ * Like u_strcasecoll_key(), but handle NULL pointers gracefully.
+ */
+char *u_strcasecoll_key0(const char *str);
+
#endif
diff --git a/utils.h b/utils.h
index 4f28ef5..f790504 100644
--- a/utils.h
+++ b/utils.h
@@ -80,6 +80,16 @@ static inline int str_to_int(const char *str, long int *val)
return 0;
}

+static inline int strcmp0(const char *str1, const char *str2)
+{
+ if (!str1)
+ return str2 ? -1 : 0;
+ if (!str2)
+ return 1;
+
+ return strcmp(str1, str2);
+}
+
static inline time_t file_get_mtime(const char *filename)
{
struct stat s;