[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [serf-dev] Re: What stands between us and branching 1.7?

From: Justin Erenkrantz <justin_at_erenkrantz.com>
Date: Sun, 23 Jan 2011 11:39:59 -0800

On Sun, Jan 23, 2011 at 10:22 AM, Lieven Govaerts <svnlgo_at_mobsol.be> wrote:
> Things have changed since then though. Can anyone test with svn 1.6.x to see
> how it uses memory?

For ra_serf, I'm wondering if we're creating an additional pool that
isn't necessary - namely the editor_pool.

I've done some light testing with the ra_serf patch below, which does
the following things:
 - Create a separate pool hierarchy for files
 - Combines the per-file editor pool with the regular pool for file

For serf itself, I've also switched zlib to use the bucket allocator -
which also helps which churn - as every call to inflateInit/inflateEnd
invokes malloc/free - so we can save on that quite bit.

Overall, this seems to reduce the amount of 20KB allocations
substantially...but not sure it does much to the overall memory usage.

Not sure if/when I'll pick this up again...so patches below if someone
else wants to run with it. -- justin

Index: update.c
===================================================================
--- update.c (revision 1062462)
+++ update.c (working copy)
@@ -107,6 +107,7 @@
   /* controlling dir baton - this is only created in open_dir() */
   void *dir_baton;
   apr_pool_t *dir_baton_pool;
+ apr_pool_t *file_pool;

   /* Our master update editor and baton. */
   const svn_delta_editor_t *update_editor;
@@ -202,9 +203,6 @@
   /* The properties for this file */
   apr_hash_t *props;

- /* pool passed to update->add_file, etc. */
- apr_pool_t *editor_pool;
-
   /* controlling file_baton and textdelta handler */
   void *file_baton;
   const char *base_checksum;
@@ -403,7 +401,7 @@
       report_info_t *new_info;

       new_info = apr_pcalloc(info_parent_pool, sizeof(*new_info));
- apr_pool_create(&new_info->pool, info_parent_pool);
+ apr_pool_create(&new_info->pool, info->dir->file_pool);
       new_info->file_baton = NULL;
       new_info->lock_token = NULL;
       new_info->fetch_file = FALSE;
@@ -500,6 +498,7 @@
   if (dir->base_name[0] == '\0')
     {
       apr_pool_create(&dir->dir_baton_pool, dir->pool);
+ apr_pool_create(&dir->file_pool, dir->pool);

       if (dir->report_context->destination &&
           dir->report_context->sess->wc_callbacks->invalidate_wc_props)
@@ -519,6 +518,7 @@
       SVN_ERR(open_dir(dir->parent_dir));

       apr_pool_create(&dir->dir_baton_pool, dir->parent_dir->dir_baton_pool);
+ dir->file_pool = dir->parent_dir->file_pool;

       if (SVN_IS_VALID_REVNUM(dir->base_rev))
         {
@@ -632,7 +632,7 @@
   if (lock_val)
     {
       char *new_lock;
- new_lock = apr_pstrdup(info->editor_pool, lock_val);
+ new_lock = apr_pstrdup(info->pool, lock_val);
       apr_collapse_spaces(new_lock, new_lock);
       lock_val = new_lock;
     }
@@ -641,7 +641,7 @@
     {
       svn_string_t *str;

- str = svn_string_ncreate("", 1, info->editor_pool);
+ str = svn_string_ncreate("", 1, info->pool);

       svn_ra_serf__set_ver_prop(info->dir->removed_props, info->base_name,
                                 info->base_rev, "DAV:", "lock-token",
@@ -756,13 +756,11 @@
           return error_fetch(request, fetch_ctx, err);
         }

- apr_pool_create(&info->editor_pool, info->dir->dir_baton_pool);
-
       /* Expand our full name now if we haven't done so yet. */
       if (!info->name)
         {
           info->name_buf = svn_stringbuf_dup(info->dir->name_buf,
- info->editor_pool);
+ info->pool);
           svn_path_add_component(info->name_buf, info->base_name);
           info->name = info->name_buf->data;
         }
@@ -772,7 +770,7 @@
           err = info->dir->update_editor->open_file(info->name,
                                                     info->dir->dir_baton,
                                                     info->base_rev,
- info->editor_pool,
+ info->pool,
                                                     &info->file_baton);
         }
       else
@@ -781,7 +779,7 @@
                                                    info->dir->dir_baton,
                                                    info->copyfrom_path,
                                                    info->copyfrom_rev,
- info->editor_pool,
+ info->pool,
                                                    &info->file_baton);
         }

@@ -792,7 +790,7 @@

       err = info->dir->update_editor->apply_textdelta(info->file_baton,
                                                       info->base_checksum,
- info->editor_pool,
+ info->pool,
                                                       &info->textdelta,
                                                       &info->textdelta_baton);

@@ -806,7 +804,7 @@
           fetch_ctx->delta_stream =
               svn_txdelta_parse_svndiff(info->textdelta,
                                         info->textdelta_baton,
- TRUE, info->editor_pool);
+ TRUE, info->pool);
         }
       else
         {
@@ -922,24 +920,24 @@
                                       info->base_name,
                                       info->base_rev,
                                       set_file_props,
- info, info->editor_pool);
+ info, info->pool);
           svn_ra_serf__walk_all_props(info->dir->removed_props,
                                       info->base_name,
                                       info->base_rev,
                                       remove_file_props,
- info, info->editor_pool);
+ info, info->pool);
           if (info->fetch_props)
             {
               svn_ra_serf__walk_all_props(info->props,
                                           info->url,
                                           info->target_rev,
                                           set_file_props,
- info, info->editor_pool);
+ info, info->pool);
             }

           err = info->dir->update_editor->close_file(info->file_baton,
                                                      info->final_checksum,
- info->editor_pool);
+ info->pool);

           if (err)
             {
@@ -953,7 +951,6 @@
           *fetch_ctx->done_list = &fetch_ctx->done_item;

           /* We're done with our pools. */
- svn_pool_destroy(info->editor_pool);
           svn_pool_destroy(info->pool);

           return status;
@@ -1055,13 +1052,11 @@
   /* Ensure our parent is open. */
   SVN_ERR(open_dir(info->dir));

- apr_pool_create(&info->editor_pool, info->dir->dir_baton_pool);
-
   /* Expand our full name now if we haven't done so yet. */
   if (!info->name)
     {
       info->name_buf = svn_stringbuf_dup(info->dir->name_buf,
- info->editor_pool);
+ info->pool);
       svn_path_add_component(info->name_buf, info->base_name);
       info->name = info->name_buf->data;
     }
@@ -1071,7 +1066,7 @@
       SVN_ERR(info->dir->update_editor->open_file(info->name,
                                                   info->dir->dir_baton,
                                                   info->base_rev,
- info->editor_pool,
+ info->pool,
                                                   &info->file_baton));
     }
   else
@@ -1080,7 +1075,7 @@
                                                  info->dir->dir_baton,
                                                  info->copyfrom_path,
                                                  info->copyfrom_rev,
- info->editor_pool,
+ info->pool,
                                                  &info->file_baton));
     }

@@ -1088,7 +1083,7 @@
     {
       SVN_ERR(info->dir->update_editor->apply_textdelta(info->file_baton,
                                                     info->base_checksum,
- info->editor_pool,
+ info->pool,
                                                     &info->textdelta,
                                                     &info->textdelta_baton));
     }
@@ -1099,22 +1094,21 @@
   /* set all of the properties we received */
   svn_ra_serf__walk_all_props(info->props,
                               info->base_name, info->base_rev,
- set_file_props, info, info->editor_pool);
+ set_file_props, info, info->pool);
   svn_ra_serf__walk_all_props(info->dir->removed_props,
                               info->base_name, info->base_rev,
- remove_file_props, info, info->editor_pool);
+ remove_file_props, info, info->pool);
   if (info->fetch_props)
     {
       svn_ra_serf__walk_all_props(info->props, info->url, info->target_rev,
- set_file_props, info, info->editor_pool);
+ set_file_props, info, info->pool);
     }

   SVN_ERR(info->dir->update_editor->close_file(info->file_baton,
                                                info->final_checksum,
- info->editor_pool));
+ info->pool));

   /* We're done with our pools. */
- svn_pool_destroy(info->editor_pool);
   svn_pool_destroy(info->pool);

   info->dir->ref_count--;

(serf)
Index: buckets/deflate_buckets.c
===================================================================
--- buckets/deflate_buckets.c (revision 1427)
+++ buckets/deflate_buckets.c (working copy)
@@ -78,6 +78,18 @@
           | (((unsigned long)string[3]) << 24);
 }

+voidpf _alloc_zlib(voidpf opaque, unsigned items, unsigned size)
+{
+ serf_bucket_alloc_t *allocator = (serf_bucket_alloc_t*)opaque;
+ return serf_bucket_mem_alloc(allocator, items * size);
+}
+
+void _free_zlib(voidpf opaque, voidpf ptr)
+{
+ serf_bucket_alloc_t *allocator = (serf_bucket_alloc_t*)opaque;
+ serf_bucket_mem_free(allocator, ptr);
+}
+
 serf_bucket_t *serf_bucket_deflate_create(
     serf_bucket_t *stream,
     serf_bucket_alloc_t *allocator,
@@ -93,6 +105,9 @@
     ctx->crc = 0;
     /* zstream must be NULL'd out. */
     memset(&ctx->zstream, 0, sizeof(ctx->zstream));
+ ctx->zstream.opaque = allocator;
+ ctx->zstream.zalloc = _alloc_zlib;
+ ctx->zstream.zfree = _free_zlib;

     switch (ctx->format) {
         case SERF_DEFLATE_GZIP:
Received on 2011-01-23 20:40:37 CET

This is an archived mail posted to the Subversion Dev mailing list.