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

[PATCH] Make ra_serf be more aggressive about closing directories in update

From: Justin Erenkrantz <justin_at_erenkrantz.com>
Date: Mon, 18 Jun 2012 08:18:44 +0200

One issue that I've come across in my recent analysis is that ra_serf
isn't always aggressive in closing directories when doing an update.
Specifically, when the directory propfind isn't received by the time
all of the files are processed, ra_serf won't look at the directory
handle again until everything is complete. (It only evaluates when
closing the files.) When doing a large checkout (like tags/), this
can be a bit problematic as we really need to be aggressive in
reclaiming all of the pools to keep our memory usage down as much as
possible.

This patch is unfinished...it solves the main issue, but there is some
corruption in the active_dir_propfinds (which I sort of tried to
address by using serf's allocator; which isn't a normal paradigm for
Subversion). When I get back home today, I need to concentrate on my
pending cross-country move and starting my new position in NYC. So,
it may be a few weeks before I get any real chance to look at this
again.

In the meantime, I know that Greg is planning to rewrite the XML
parsing in update.c to fit the new parsing paradigm that has been
introduced into other parts of ra_serf as well as getting the Propfind
depth:1 in. My guess it that after the parsing rewrite and depth 1 is
complete, this issue may fall by the wayside...or not.

Hence, I'll just throw the current patch on the list for posterity.
When I come up for air again, I'll try to evaluate if it is still
needed. FWIW, Instruments on OS X is awesome. -- justin

Index: subversion/libsvn_ra_serf/update.c
===================================================================
--- subversion/libsvn_ra_serf/update.c (revision 1351209)
+++ subversion/libsvn_ra_serf/update.c (working copy)
@@ -48,6 +48,8 @@
 #include "ra_serf.h"
 #include "../libsvn_ra/ra_loader.h"

+#include "serf_bucket_util.h"
+
 
 /*
  * This enum represents the current state of our XML parsing for a REPORT.
@@ -292,6 +294,7 @@ typedef struct report_fetch_t {
  */
 struct report_context_t {
   apr_pool_t *pool;
+ serf_bucket_alloc_t *allocator;

   svn_ra_serf__session_t *sess;
   svn_ra_serf__connection_t *conn;
@@ -348,7 +351,11 @@ struct report_context_t {

   /* completed PROPFIND requests (contains svn_ra_serf__handler_t) */
   svn_ra_serf__list_t *done_propfinds;
+ svn_ra_serf__list_t *done_dir_propfinds;

+ /* list of outstanding prop changes (contains report_dir_t) */
+ svn_ra_serf__list_t *active_dir_propfinds;
+
   /* list of files that only have prop changes (contains report_info_t) */
   svn_ra_serf__list_t *file_propchanges_only;

@@ -654,6 +661,7 @@ close_dir(report_dir_t *dir)
     }

   svn_pool_destroy(dir->dir_baton_pool);
+ printf("destroy: %p (%s)\n", dir->pool, dir->name);
   svn_pool_destroy(dir->pool);

   return SVN_NO_ERROR;
@@ -1888,6 +1896,7 @@ end_report(svn_ra_serf__xml_parser_t *parser,
        */
       if (!SVN_IS_VALID_REVNUM(info->dir->base_rev) || info->dir->fetch_props)
         {
+ svn_ra_serf__list_t *list_item;
           /* Unconditionally set fetch_props now. */
           info->dir->fetch_props = TRUE;

@@ -1897,7 +1906,7 @@ end_report(svn_ra_serf__xml_parser_t *parser,
                                              info->dir->url,
                                              ctx->target_rev, "0",
                                              all_props,
- &ctx->done_propfinds,
+ &ctx->done_dir_propfinds,
                                              info->dir->pool));
           SVN_ERR_ASSERT(info->dir->propfind_handler);

@@ -1906,6 +1915,11 @@ end_report(svn_ra_serf__xml_parser_t *parser,

           ctx->active_propfinds++;

+ list_item = serf_bucket_mem_alloc(ctx->allocator,
sizeof(*list_item));
+ list_item->data = info->dir;
+ list_item->next = ctx->active_dir_propfinds;
+ ctx->active_dir_propfinds = list_item;
+
           if (ctx->active_fetches + ctx->active_propfinds
> REQUEST_COUNT_TO_PAUSE)
             ctx->parser_ctx->paused = TRUE;
@@ -2525,12 +2539,77 @@ finish_report(void *report_baton,
         }
       report->done_propfinds = NULL;

+ done_list = report->done_dir_propfinds;
+ while (done_list)
+ {
+ svn_pool_clear(iterpool_inner);
+
+ report->active_propfinds--;
+
+ if (report->active_dir_propfinds)
+ {
+ svn_ra_serf__list_t *cur, *prev;
+
+ cur = report->active_dir_propfinds;
+ prev = NULL;
+
+ while (cur)
+ {
+ report_dir_t *item = cur->data;
+
+ if (item->propfind_handler == done_list->data)
+ {
+ break;
+ }
+
+ prev = cur;
+ cur = cur->next;
+ }
+ if (cur)
+ {
+ report_dir_t *cur_dir = cur->data;
+
+ while (cur_dir && !cur_dir->ref_count && cur_dir->tag_closed
+ && (!cur_dir->fetch_props
+ || cur_dir->propfind_handler->done))
+ {
+ printf("clear: %p : %s\n", cur_dir, cur_dir->name);
+ report_dir_t *parent = cur_dir->parent_dir;
+
+ SVN_ERR(close_dir(cur_dir));
+ if (parent)
+ {
+ parent->ref_count--;
+ }
+ else
+ {
+ closed_root = TRUE;
+ }
+ cur_dir = parent;
+ }
+
+ if (!prev)
+ {
+ report->active_dir_propfinds = cur->next;
+ }
+ else
+ {
+ prev->next = cur->next;
+ }
+ serf_bucket_mem_free(report->allocator, cur);
+ }
+ }
+
+ done_list = done_list->next;
+ }
+ report->done_dir_propfinds = NULL;
+
       /* prune our fetches list if they are done. */
       done_list = report->done_fetches;
       while (done_list)
         {
           report_fetch_t *done_fetch = done_list->data;
- report_dir_t *cur_dir;
+ report_dir_t *cur_dir, *t_d;

           /* decrease our parent's directory refcount. */
           cur_dir = done_fetch->info->dir;
@@ -2549,6 +2628,17 @@ finish_report(void *report_baton,
            * we've already completed the propfind
            * then, we know it's time for us to close this directory.
            */
+ t_d = cur_dir;
+ while (t_d) {
+ printf("%p : %s %ld ", t_d, t_d->name, t_d->ref_count);
+ if (t_d->ref_count) {
+ printf("\n");
+ } else {
+ printf("-###- %d %d %p %d\n", t_d->tag_closed,
t_d->fetch_props, t_d->propfind_handler, t_d->propfind_handler ?
t_d->propfind_handler->done : -1);
+ }
+ t_d = t_d->parent_dir;
+ }
+
           while (cur_dir && !cur_dir->ref_count && cur_dir->tag_closed
                  && (!cur_dir->fetch_props
                      || cur_dir->propfind_handler->done))
@@ -2674,6 +2764,7 @@ make_update_reporter(svn_ra_session_t *ra_session,

   report = apr_pcalloc(result_pool, sizeof(*report));
   report->pool = result_pool;
+ report->allocator = serf_bucket_allocator_create(report->pool, NULL, NULL);
   report->sess = sess;
   report->conn = report->sess->conns[0];
   report->target_rev = revision;
Received on 2012-06-18 08:19:24 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.