Commit 6b59d167 authored by W. Michael Petullo's avatar W. Michael Petullo
Browse files

Significant work on reducing memory usage while building "/1/items" response



We previously simply called foreach...add_entry_to_mlcl and later
serialized the entire structure. This has the disadvantage that the entire
response must be in memory before libsoup sends it to the client. Now,
we transmit portions at a time.
Signed-off-by: W. Michael Petullo's avatarW. Michael Petullo <mike@flyn.org>
parent 4b5e62e1
07 November 2010 W. Michael Petullo <mike@flyn.org>
* Significant work on an interim solution for the problem
of memory usage while building the "/1/items" response. We
previously simply called foreach...add_entry_to_mlcl and later
serialized the entire structure. This has the disadvantage that
the entire response must be in memory before libsoup sends it
to the client. Now, we transmit portions at a time.
31 October 2010 W. Michael Petullo <mike@flyn.org> 31 October 2010 W. Michael Petullo <mike@flyn.org>
* Bump version number. * Bump version number.
......
This documents the effort to make dmapd and libdmapsharing more memory This documents the effort to make dmapd and libdmapsharing more memory
efficient. efficient. This document describes work done in both libdmapsharing and
dmapd, because dmapd is the primary application of libdmapsharing's
server-side functionality.
I found that dmapd used a significant amount of heap space while trying to I found that dmapd used a significant amount of heap space while trying to
port dmapd to OpenWRT on a WRT160NL router with 32 Megabytes of RAM. A port dmapd to OpenWRT on a WRT160NL router with 32 Megabytes of RAM. A
...@@ -37,7 +39,7 @@ efficient: ...@@ -37,7 +39,7 @@ efficient:
Database implementation. This has not resulted in any improvement, Database implementation. This has not resulted in any improvement,
so the hash table implementation is once again the default. so the hash table implementation is once again the default.
2. Fixed several memory leaks after analysing with valgrind. 2. Fixed several memory leaks after analyzing with valgrind.
Heap without client connection: 3,940,352 bytes Heap without client connection: 3,940,352 bytes
...@@ -57,3 +59,24 @@ GSList of ID's. ...@@ -57,3 +59,24 @@ GSList of ID's.
Heap without client connection: 3,518,464 bytes Heap without client connection: 3,518,464 bytes
Heap after client connection and song list: 6,864,896 bytes Heap after client connection and song list: 6,864,896 bytes
Max heap usages (massif): 8,945,760 bytes Max heap usages (massif): 8,945,760 bytes
6. Modify libdmapsharing so that it responds to "/1/items" requests in
chunks, avoiding the need to build the entire response in memory.
The effectiveness of this can be measured by observing the
maximum heap usage after a client has asked for a full media
list. Previous to this change, dmapd would use 7,811,072 bytes
of heap to describe a 2,232-song library. After this change,
dmapd's usage was 3,665,920 bytes, a very small increase over
the memory used by dmapd immediately after starting.
7. Write a disk-based database for dmapd.
In order to minimize the memory used by a newly started dmapd,
I wrote a disk-based DMAPDb backend. This backend stores all
database data in files and avoids storing the entire database
in memory. To measure the effectiveness, I measured the memory
usage of dmapd after loading a 3,276-photograph library (note
that thumbnails are especially problematic). The GHashTable-based
database used 35,885,056 bytes of heap space. The disk-based
database used 4,763,648 bytes.
Reduce memory needed to send entire listing to client: Reduce memory needed to send entire listing to client:
Can we predict length of mlcl? get bdb dmapd backend working well
dmapd-structure.c: what does this mean in the case of MLCL: guint32 size = GINT32_TO_BE (item->size); review changes
dmap-share.c.../1/items use chunked response see daap-share.c:databases_items_xxx() / send_chunked_file()
collect ID's into list
header-complete callback, send preamble
chunk-complete callback (modify add_entry_to_mlcl into callback) take ID list as user_data
for each callback, pull an ID off list and write it
once list is empty, done
SHOULD THIS STUFF BE USED IN NEW SCHEME? HOW TO HANDLE SERIALIZING MLCL?
eventually, get rid of?
dmap_structure_serialize
dmap_structure_serialize_node
dmap_structure_add
Fix DNSSD on Mac OS X Fix DNSSD on Mac OS X
......
...@@ -76,7 +76,7 @@ static void databases_items_xxx (DMAPShare *share, ...@@ -76,7 +76,7 @@ static void databases_items_xxx (DMAPShare *share,
GHashTable *query, GHashTable *query,
SoupClientContext *context); SoupClientContext *context);
static struct DMAPMetaDataMap *get_meta_data_map (DMAPShare *share); static struct DMAPMetaDataMap *get_meta_data_map (DMAPShare *share);
static void add_entry_to_mlcl (gpointer id, static guint32 add_entry_to_mlcl (gpointer id,
DMAPRecord *record, DMAPRecord *record,
gpointer mb); gpointer mb);
...@@ -542,7 +542,7 @@ send_chunked_file (SoupServer *server, SoupMessage *message, DAAPRecord *record, ...@@ -542,7 +542,7 @@ send_chunked_file (SoupServer *server, SoupMessage *message, DAAPRecord *record,
/* NOTE: cd g_free'd by chunked_message_finished(). */ /* NOTE: cd g_free'd by chunked_message_finished(). */
} }
static void static guint32
add_entry_to_mlcl (gpointer id, add_entry_to_mlcl (gpointer id,
DMAPRecord *record, DMAPRecord *record,
gpointer _mb) gpointer _mb)
...@@ -714,7 +714,7 @@ add_entry_to_mlcl (gpointer id, ...@@ -714,7 +714,7 @@ add_entry_to_mlcl (gpointer id,
dmap_structure_add (mlit, DMAP_CC_AEMK, mediakind); dmap_structure_add (mlit, DMAP_CC_AEMK, mediakind);
} }
return; return ((DMAPStructureItem *) mlit->data)->size;
} }
static void static void
......
...@@ -75,7 +75,9 @@ typedef unsigned long long bitwise; ...@@ -75,7 +75,9 @@ typedef unsigned long long bitwise;
struct MLCL_Bits { struct MLCL_Bits {
GNode *mlcl; GNode *mlcl;
bitwise bits; bitwise bits;
gpointer pointer; /* FIXME: yuck, used for accumulater: */
gpointer user_data1;
gpointer user_data2;
}; };
typedef enum { typedef enum {
......
...@@ -91,6 +91,11 @@ struct DMAPSharePrivate { ...@@ -91,6 +91,11 @@ struct DMAPSharePrivate {
GHashTable *session_ids; GHashTable *session_ids;
}; };
struct share_bitwise_t {
DMAPShare *share;
bitwise bits;
};
static void dmap_share_init (DMAPShare *share); static void dmap_share_init (DMAPShare *share);
static void dmap_share_class_init (DMAPShareClass *klass); static void dmap_share_class_init (DMAPShareClass *klass);
...@@ -1444,6 +1449,81 @@ _dmap_share_ctrl_int (DMAPShare *share, ...@@ -1444,6 +1449,81 @@ _dmap_share_ctrl_int (DMAPShare *share,
g_debug ("ctrl-int not implemented"); g_debug ("ctrl-int not implemented");
} }
static void
accumulate_mlcl_size (gpointer id,
DMAPRecord *record,
gpointer mb)
{
/* Make copy and set mlcl to NULL so real MLCL does not get changed */
struct MLCL_Bits mb_copy = *((struct MLCL_Bits *) mb);
mb_copy.mlcl = dmap_structure_add (NULL, DMAP_CC_MLCL);;
*((guint *) ((struct MLCL_Bits *) mb)->user_data1) += DMAP_SHARE_GET_CLASS (((struct MLCL_Bits *) mb)->user_data2)->add_entry_to_mlcl (id, record, &mb_copy);
/* Destroy created structures as we go. */
dmap_structure_destroy (mb_copy.mlcl);
}
static void
accumulate_ids (gpointer id,
DMAPRecord *record,
GSList **list)
{
*list = g_slist_append (*list, id);
}
static void
write_daap_preamble (SoupMessage *message, GNode *node)
{
guint length;
gchar *data = dmap_structure_serialize (node, &length);
soup_message_body_append (message->response_body,
SOUP_MEMORY_TAKE,
data,
length);
dmap_structure_destroy (node);
}
static void
write_next_mlit (SoupMessage *message, struct share_bitwise_t *share_bitwise)
{
gchar *data;
guint length;
DMAPRecord *record;
static GSList *id_list = NULL;
struct MLCL_Bits mb = {NULL,0};
if (id_list == NULL) {
g_debug ("Initializing ID list.");
dmap_db_foreach (share_bitwise->share->priv->db, (GHFunc) accumulate_ids, &id_list);
}
record = dmap_db_lookup_by_id (share_bitwise->share->priv->db, GPOINTER_TO_UINT (id_list->data));
mb.bits = share_bitwise->bits;
mb.mlcl = dmap_structure_add (NULL, DMAP_CC_MLCL);
DMAP_SHARE_GET_CLASS (share_bitwise->share)->add_entry_to_mlcl (id_list->data, record, &mb);
data = dmap_structure_serialize (g_node_first_child(mb.mlcl), &length);
soup_message_body_append (message->response_body,
SOUP_MEMORY_TAKE,
data,
length);
g_debug ("Sending ID %u.", GPOINTER_TO_UINT (id_list->data));
dmap_structure_destroy (mb.mlcl);
id_list = g_slist_remove (id_list, id_list->data);
if (id_list == NULL) {
g_debug ("No more ID's, sending message complete.");
soup_message_body_complete (message->response_body);
g_free (share_bitwise);
}
g_object_unref (record);
soup_server_unpause_message (share_bitwise->share->priv->server, message);
}
void void
_dmap_share_databases (DMAPShare *share, _dmap_share_databases (DMAPShare *share,
SoupServer *server, SoupServer *server,
...@@ -1623,18 +1703,64 @@ _dmap_share_databases (DMAPShare *share, ...@@ -1623,18 +1703,64 @@ _dmap_share_databases (DMAPShare *share,
dmap_structure_add (adbs, DMAP_CC_MUTY, 0); dmap_structure_add (adbs, DMAP_CC_MUTY, 0);
dmap_structure_add (adbs, DMAP_CC_MTCO, (gint32) num_songs); dmap_structure_add (adbs, DMAP_CC_MTCO, (gint32) num_songs);
dmap_structure_add (adbs, DMAP_CC_MRCO, (gint32) num_songs); dmap_structure_add (adbs, DMAP_CC_MRCO, (gint32) num_songs);
mb.mlcl = dmap_structure_add (adbs, DMAP_CC_MLCL);
if (record_query) { if (record_query) {
/* NOTE: still uses old technique: */
mb.mlcl = dmap_structure_add (adbs, DMAP_CC_MLCL); // Was shared with else before
g_hash_table_foreach (records, (GHFunc) DMAP_SHARE_GET_CLASS (share)->add_entry_to_mlcl, &mb); g_hash_table_foreach (records, (GHFunc) DMAP_SHARE_GET_CLASS (share)->add_entry_to_mlcl, &mb);
g_hash_table_destroy (records); g_hash_table_destroy (records);
_dmap_share_message_set_from_dmap_structure (share, message, adbs); // Was shared with else before
dmap_structure_destroy (adbs); // Was shared with else before
} else { } else {
dmap_db_foreach (share->priv->db, (GHFunc) DMAP_SHARE_GET_CLASS (share)->add_entry_to_mlcl, &mb); /* NOTE:
* We previously simply called foreach...add_entry_to_mlcl and later serialized the entire
* structure. This has the disadvantage that the entire response must be in memory before
* libsoup sends it to the client.
*
* Now, we go through the database in multiple passes (as an interim solution):
*
* 1. Accumulate the eventual size of the MLCL by creating and then free'ing each MLIT.
* 2. Generate the DAAP preamble ending with the MLCL (with size fudged for ADBS and MLCL).
* 3. Setup libsoup response headers, etc.
* 4. Setup callback to transmit DAAP preamble (write_daap_preamble)
* 5. Setup callback to transmit MLIT's (write_next_mlit)
* NOTE: write_next_mlit uses some tricks to iterate through all record ID's
*/
struct share_bitwise_t *share_bitwise;
/* 1: */
/* FIXME: user_data1/2 is ugly: */
guint32 size = 0;
mb.user_data1 = &size;
mb.user_data2 = share;
dmap_db_foreach (share->priv->db, (GHFunc) accumulate_mlcl_size, &mb);
/* 2: */
mb.mlcl = dmap_structure_add (adbs, DMAP_CC_MLCL);
dmap_structure_set_predicted_size (adbs, dmap_structure_get_size(adbs) + size);
dmap_structure_set_predicted_size (mb.mlcl, dmap_structure_get_size(mb.mlcl) + size);
/* 3: */
soup_message_set_status (message, SOUP_STATUS_OK);
soup_message_headers_set_content_length (message->response_headers, dmap_structure_get_size(adbs) + 8);
/* Free memory after each chunk sent out over network. */
soup_message_body_set_accumulate (message->response_body, FALSE);
soup_message_headers_append (message->response_headers, "Connection", "Close");
soup_message_headers_append (message->response_headers, "Content-Type", "application/x-daap-tagged");
DMAP_SHARE_GET_CLASS (share)->message_add_standard_headers (share, message);
/* 4: */
g_signal_connect (message, "wrote_headers", G_CALLBACK (write_daap_preamble), adbs);
/* 5: */
share_bitwise = g_new (struct share_bitwise_t, 1);
share_bitwise->share = share;
share_bitwise->bits = mb.bits;
g_signal_connect (message, "wrote_chunk", G_CALLBACK (write_next_mlit), share_bitwise);
} }
_dmap_share_message_set_from_dmap_structure (share, message, adbs);
dmap_structure_destroy (adbs);
adbs = NULL;
} else if (g_ascii_strcasecmp ("/1/containers", rest_of_path) == 0) { } else if (g_ascii_strcasecmp ("/1/containers", rest_of_path) == 0) {
/* APLY database playlists /* APLY database playlists
* MSTT status * MSTT status
......
...@@ -112,7 +112,7 @@ typedef struct { ...@@ -112,7 +112,7 @@ typedef struct {
void (*message_add_standard_headers) (DMAPShare *share, void (*message_add_standard_headers) (DMAPShare *share,
SoupMessage *msg); SoupMessage *msg);
struct DMAPMetaDataMap * (*get_meta_data_map) (DMAPShare *share); struct DMAPMetaDataMap * (*get_meta_data_map) (DMAPShare *share);
void (*add_entry_to_mlcl) (gpointer id, guint32 (*add_entry_to_mlcl) (gpointer id,
DMAPRecord *record, DMAPRecord *record,
gpointer mb); gpointer mb);
void (*databases_browse_xxx) (DMAPShare *share, void (*databases_browse_xxx) (DMAPShare *share,
......
...@@ -785,3 +785,16 @@ dmap_structure_print (GNode *structure) ...@@ -785,3 +785,16 @@ dmap_structure_print (GNode *structure)
g_node_traverse (structure, G_PRE_ORDER, G_TRAVERSE_ALL, -1, (GNodeTraverseFunc)print_dmap_item, NULL); g_node_traverse (structure, G_PRE_ORDER, G_TRAVERSE_ALL, -1, (GNodeTraverseFunc)print_dmap_item, NULL);
} }
} }
guint
dmap_structure_get_size (GNode *structure)
{
return ((DMAPStructureItem *) structure->data)->size;
}
void
dmap_structure_set_predicted_size (GNode *structure, guint size)
{
((DMAPStructureItem *) structure->data)->size = size;
}
...@@ -209,6 +209,8 @@ GNode *dmap_structure_find_node (GNode *structure, ...@@ -209,6 +209,8 @@ GNode *dmap_structure_find_node (GNode *structure,
DMAPContentCode code); DMAPContentCode code);
void dmap_structure_print (GNode *structure); void dmap_structure_print (GNode *structure);
void dmap_structure_destroy (GNode *structure); void dmap_structure_destroy (GNode *structure);
guint dmap_structure_get_size (GNode *structure);
void dmap_structure_set_predicted_size (GNode *structure, guint size);
typedef enum { typedef enum {
DMAP_TYPE_BYTE = 0x0001, DMAP_TYPE_BYTE = 0x0001,
......
...@@ -78,7 +78,7 @@ static void databases_items_xxx (DMAPShare *share, ...@@ -78,7 +78,7 @@ static void databases_items_xxx (DMAPShare *share,
GHashTable *query, GHashTable *query,
SoupClientContext *context); SoupClientContext *context);
static struct DMAPMetaDataMap *get_meta_data_map (DMAPShare *share); static struct DMAPMetaDataMap *get_meta_data_map (DMAPShare *share);
static void add_entry_to_mlcl (gpointer id, static guint32 add_entry_to_mlcl (gpointer id,
DMAPRecord *record, DMAPRecord *record,
gpointer mb); gpointer mb);
...@@ -347,7 +347,7 @@ file_to_mmap (const char *location) ...@@ -347,7 +347,7 @@ file_to_mmap (const char *location)
return mapped_file; return mapped_file;
} }
static void static guint32
add_entry_to_mlcl (gpointer id, add_entry_to_mlcl (gpointer id,
DMAPRecord *record, DMAPRecord *record,
gpointer _mb) gpointer _mb)
...@@ -466,7 +466,8 @@ add_entry_to_mlcl (gpointer id, ...@@ -466,7 +466,8 @@ add_entry_to_mlcl (gpointer id,
} }
dmap_structure_add (mlit, DMAP_CC_PFDT, data, size); dmap_structure_add (mlit, DMAP_CC_PFDT, data, size);
} }
return;
return ((DMAPStructureItem *) mlit->data)->size;
} }
static void static void
......
...@@ -233,9 +233,9 @@ test_daap_record_class_init (TestDAAPRecordClass *klass) ...@@ -233,9 +233,9 @@ test_daap_record_class_init (TestDAAPRecordClass *klass)
g_object_class_override_property (gobject_class, PROP_LOCATION, "location"); g_object_class_override_property (gobject_class, PROP_LOCATION, "location");
g_object_class_override_property (gobject_class, PROP_TITLE, "title"); g_object_class_override_property (gobject_class, PROP_TITLE, "title");
g_object_class_override_property (gobject_class, PROP_ALBUM, "album"); g_object_class_override_property (gobject_class, PROP_ALBUM, "songalbum");
g_object_class_override_property (gobject_class, PROP_ARTIST, "artist"); g_object_class_override_property (gobject_class, PROP_ARTIST, "songartist");
g_object_class_override_property (gobject_class, PROP_GENRE, "genre"); g_object_class_override_property (gobject_class, PROP_GENRE, "songgenre");
g_object_class_override_property (gobject_class, PROP_FORMAT, "format"); g_object_class_override_property (gobject_class, PROP_FORMAT, "format");
g_object_class_override_property (gobject_class, PROP_RATING, "rating"); g_object_class_override_property (gobject_class, PROP_RATING, "rating");
g_object_class_override_property (gobject_class, PROP_FILESIZE, "filesize"); g_object_class_override_property (gobject_class, PROP_FILESIZE, "filesize");
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment