<p>George Joseph <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/7453">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Richard Mudgett: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, but someone else must approve
George Joseph: Looks good to me, approved; Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">media_index: Improve startup.<br><br>This eliminates some wasteful operations in media_index startup.<br><br>* Replace statically set string-fields with char[0].<br>* Eliminate pointless RAII_VAR's.<br>* alloc_variant: Avoid pointless ao2_find on new info->variant.<br>* Stop trying find_variant before alloc_variant.<br>* process_media_file: replace ast_str with ast_asprintf. This avoids<br> reallocation of file_id_str.<br><br>Overall sounds_index.c is about 27% of Asterisk startup time when using<br>sample configs. This patch reduces it to 20%. This is a half-fix. The<br>real problem is that the media_index is regenerated repeatedly - 68<br>times in my test.<br><br>Change-Id: Ia50b752f8efb356f852b05c4be495a6631af8652<br>---<br>M main/media_index.c<br>1 file changed, 58 insertions(+), 63 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/main/media_index.c b/main/media_index.c<br>index 3e38245..60bdfe3 100644<br>--- a/main/media_index.c<br>+++ b/main/media_index.c<br>@@ -45,10 +45,10 @@<br> /*! \brief Structure to hold a list of the format variations for a media file for a specific variant */<br> struct media_variant {<br> AST_DECLARE_STRING_FIELDS(<br>- AST_STRING_FIELD(variant); /*!< The variant this media is available in */<br> AST_STRING_FIELD(description); /*!< The description of the media */<br> );<br> struct ast_format_cap *formats; /*!< The formats this media is available in for this variant */<br>+ char variant[0]; /*!< The variant this media is available in */<br> };<br> <br> static void media_variant_destroy(void *obj)<br>@@ -61,20 +61,23 @@<br> <br> static struct media_variant *media_variant_alloc(const char *variant_str)<br> {<br>- RAII_VAR(struct media_variant *, variant, ao2_alloc(sizeof(*variant), media_variant_destroy), ao2_cleanup);<br>+ size_t str_sz = strlen(variant_str) + 1;<br>+ struct media_variant *variant;<br> <br>- if (!variant || ast_string_field_init(variant, 8)) {<br>+ variant = ao2_alloc(sizeof(*variant) + str_sz, media_variant_destroy);<br>+ if (!variant) {<br> return NULL;<br> }<br>+<br>+ memcpy(variant->variant, variant_str, str_sz);<br> <br> variant->formats = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);<br>- if (!variant->formats) {<br>+ if (!variant->formats || ast_string_field_init(variant, 8)) {<br>+ ao2_ref(variant, -1);<br>+<br> return NULL;<br> }<br> <br>- ast_string_field_set(variant, variant, variant_str);<br>-<br>- ao2_ref(variant, 1);<br> return variant;<br> }<br> <br>@@ -93,37 +96,35 @@<br> <br> /*! \brief Structure to hold information about a media file */<br> struct media_info {<br>- AST_DECLARE_STRING_FIELDS(<br>- AST_STRING_FIELD(name); /*!< The file name of the media */<br>- );<br>- struct ao2_container *variants; /*!< The variants for which this media is available */<br>+ struct ao2_container *variants; /*!< The variants for which this media is available */<br>+ char name[0]; /*!< The file name of the media */<br> };<br> <br> static void media_info_destroy(void *obj)<br> {<br> struct media_info *info = obj;<br> <br>- ast_string_field_free_memory(info);<br> ao2_cleanup(info->variants);<br>- info->variants = NULL;<br> }<br> <br> static struct media_info *media_info_alloc(const char *name)<br> {<br>- RAII_VAR(struct media_info *, info, ao2_alloc(sizeof(*info), media_info_destroy), ao2_cleanup);<br>+ size_t name_sz = strlen(name) + 1;<br>+ struct media_info *info = ao2_alloc(sizeof(*info) + name_sz, media_info_destroy);<br> <br>- if (!info || ast_string_field_init(info, 128)) {<br>+ if (!info) {<br> return NULL;<br> }<br>+<br>+ memcpy(info->name, name, name_sz);<br> <br> info->variants = ao2_container_alloc(VARIANT_BUCKETS, media_variant_hash, media_variant_cmp);<br> if (!info->variants) {<br>+ ao2_ref(info, -1);<br>+<br> return NULL;<br> }<br> <br>- ast_string_field_set(info, name, name);<br>-<br>- ao2_ref(info, 1);<br> return info;<br> }<br> <br>@@ -141,38 +142,37 @@<br> }<br> <br> struct ast_media_index {<br>- AST_DECLARE_STRING_FIELDS(<br>- AST_STRING_FIELD(base_dir); /*!< Base directory for indexing */<br>- );<br> struct ao2_container *index; /*!< The index of media that has requested */<br> struct ao2_container *media_list_cache; /*!< Cache of filenames to prevent them from being regenerated so often */<br>+ char base_dir[0]; /*!< Base directory for indexing */<br> };<br> <br> static void media_index_dtor(void *obj)<br> {<br> struct ast_media_index *index = obj;<br>+<br> ao2_cleanup(index->index);<br>- index->index = NULL;<br> ao2_cleanup(index->media_list_cache);<br>- index->media_list_cache = NULL;<br>- ast_string_field_free_memory(index);<br> }<br> <br> struct ast_media_index *ast_media_index_create(const char *base_dir)<br> {<br>- RAII_VAR(struct ast_media_index *, index, ao2_alloc(sizeof(*index), media_index_dtor), ao2_cleanup);<br>- if (!index || ast_string_field_init(index, 64)) {<br>+ size_t base_dir_sz = strlen(base_dir) + 1;<br>+ struct ast_media_index *index = ao2_alloc(sizeof(*index) + base_dir_sz, media_index_dtor);<br>+<br>+ if (!index) {<br> return NULL;<br> }<br> <br>- ast_string_field_set(index, base_dir, base_dir);<br>+ memcpy(index->base_dir, base_dir, base_dir_sz);<br> <br> index->index = ao2_container_alloc(INDEX_BUCKETS, media_info_hash, media_info_cmp);<br> if (!index->index) {<br>+ ao2_ref(index, -1);<br>+<br> return NULL;<br> }<br> <br>- ao2_ref(index, +1);<br> return index;<br> }<br> <br>@@ -191,8 +191,8 @@<br> /*! \brief create the appropriate media_variant and any necessary structures */<br> static struct media_variant *alloc_variant(struct ast_media_index *index, const char *filename, const char *variant_str)<br> {<br>- RAII_VAR(struct media_info *, info, NULL, ao2_cleanup);<br>- RAII_VAR(struct media_variant *, variant, NULL, ao2_cleanup);<br>+ struct media_info *info;<br>+ struct media_variant *variant = NULL;<br> <br> info = ao2_find(index->index, filename, OBJ_KEY);<br> if (!info) {<br>@@ -204,21 +204,21 @@<br> }<br> <br> ao2_link(index->index, info);<br>+ } else {<br>+ variant = ao2_find(info->variants, variant_str, OBJ_KEY);<br> }<br> <br>- variant = ao2_find(info->variants, variant_str, OBJ_KEY);<br> if (!variant) {<br> /* This is the first time the index has seen this variant for<br> * this filename, allocate and link */<br> variant = media_variant_alloc(variant_str);<br>- if (!variant) {<br>- return NULL;<br>+ if (variant) {<br>+ ao2_link(info->variants, variant);<br> }<br>-<br>- ao2_link(info->variants, variant);<br> }<br> <br>- ao2_ref(variant, +1);<br>+ ao2_ref(info, -1);<br>+<br> return variant;<br> }<br> <br>@@ -324,15 +324,16 @@<br> /*! \brief Update an index with new format/variant information */<br> static int update_file_format_info(struct ast_media_index *index, const char *filename, const char *variant_str, struct ast_format *file_format)<br> {<br>- RAII_VAR(struct media_variant *, variant, find_variant(index, filename, variant_str), ao2_cleanup);<br>+ struct media_variant *variant;<br>+<br>+ variant = alloc_variant(index, filename, variant_str);<br> if (!variant) {<br>- variant = alloc_variant(index, filename, variant_str);<br>- if (!variant) {<br>- return -1;<br>- }<br>+ return -1;<br> }<br> <br> ast_format_cap_append(variant->formats, file_format, 0);<br>+ ao2_ref(variant, -1);<br>+<br> return 0;<br> }<br> <br>@@ -341,7 +342,8 @@<br> {<br> struct ast_format *file_format;<br> const char *file_identifier = filename_stripped;<br>- RAII_VAR(struct ast_str *, file_id_str, NULL, ast_free);<br>+ char *file_id_str = NULL;<br>+ int res;<br> <br> file_format = ast_get_format_for_file_ext(ext);<br> if (!file_format) {<br>@@ -351,19 +353,17 @@<br> <br> /* handle updating the file information */<br> if (subdir) {<br>- file_id_str = ast_str_create(64);<br>- if (!file_id_str) {<br>+ if (ast_asprintf(&file_id_str, "%s/%s", subdir, filename_stripped) == -1) {<br> return -1;<br> }<br> <br>- ast_str_set(&file_id_str, 0, "%s/%s", subdir, filename_stripped);<br>- file_identifier = ast_str_buffer(file_id_str);<br>+ file_identifier = file_id_str;<br> }<br> <br>- if (update_file_format_info(index, file_identifier, variant, file_format)) {<br>- return -1;<br>- }<br>- return 0;<br>+ res = update_file_format_info(index, file_identifier, variant, file_format);<br>+ ast_free(file_id_str);<br>+<br>+ return res;<br> }<br> <br> /*!<br>@@ -443,20 +443,18 @@<br> } else {<br> /* if there's text in cumulative_description, archive it and start anew */<br> if (file_id_persist && !ast_strlen_zero(ast_str_buffer(cumulative_description))) {<br>- RAII_VAR(struct media_variant *, variant, NULL, ao2_cleanup);<br>+ struct media_variant *variant;<br> <br>- variant = find_variant(index, file_id_persist, variant_str);<br>+ variant = alloc_variant(index, file_id_persist, variant_str);<br> if (!variant) {<br>- variant = alloc_variant(index, file_id_persist, variant_str);<br>- if (!variant) {<br>- res = -1;<br>- break;<br>- }<br>+ res = -1;<br>+ break;<br> }<br> <br> ast_string_field_set(variant, description, ast_str_buffer(cumulative_description));<br> <br> ast_str_reset(cumulative_description);<br>+ ao2_ref(variant, -1);<br> }<br> <br> ast_free(file_id_persist);<br>@@ -468,15 +466,12 @@<br> <br> /* handle the last one */<br> if (file_id_persist && !ast_strlen_zero(ast_str_buffer(cumulative_description))) {<br>- RAII_VAR(struct media_variant *, variant, NULL, ao2_cleanup);<br>+ struct media_variant *variant;<br> <br>- variant = find_variant(index, file_id_persist, variant_str);<br>- if (!variant) {<br>- variant = alloc_variant(index, file_id_persist, variant_str);<br>- }<br>-<br>+ variant = alloc_variant(index, file_id_persist, variant_str);<br> if (variant) {<br> ast_string_field_set(variant, description, ast_str_buffer(cumulative_description));<br>+ ao2_ref(variant, -1);<br> } else {<br> res = -1;<br> }<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/7453">change 7453</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/7453"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Ia50b752f8efb356f852b05c4be495a6631af8652 </div>
<div style="display:none"> Gerrit-Change-Number: 7453 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>