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

Re: svn commit: rev 3215

From: Karl Fogel <kfogel_at_newton.ch.collab.net>
Date: 2002-09-24 15:51:23 CEST

philip@tigris.org writes:
> New Revision: 3215
>
> Modified:
> branches/issue-749-caching/subversion/libsvn_wc/entries.c
> Log:
> Better use of pools. This dramatically improves memory use, it may even
> be useable now.
>
> * subversion/libsvn_wc/entries.c (svn_wc__entries_write): Don't use the
> access baton pool for writing the entries file.

Philip, I tweaked this log message to name the correct function, and
also to go into a bit more detail about the change, since I did a
double-take when first reading over the diff :-). Here's how it reads
now:

   -------------------------------------------------------------------
   Better use of pools. This dramatically improves memory use, it may
   even be useable now.

   * subversion/libsvn_wc/entries.c (svn_wc__entries_modify): Stop
     shadowing the caller pool with the access baton pool; instead,
     use the access baton pool explicitly where needed, but use the
     caller pool for svn_wc__entries_write().
   -------------------------------------------------------------------

I'm still a bit confused about the pool semantics in this function,
though, and I have a feeling that if you explain it, a lot about
access batons will become clear...

The caller `pool' is now only used twice in the function, once at the
top:

  /* Load PATH's whole entries file. */
  SVN_ERR (svn_wc_entries_read (&entries, adm_access, TRUE, pool));

and once at the bottom:

  /* Sync changes to disk. */
  SVN_ERR (svn_wc__entries_write (entries, adm_access, pool));

In between, every function that needs a pool is getting

   svn_wc_adm_access_pool (adm_access)

as its pool argument, even though none of those functions take
`adm_access' itself. So I'm wondering, why don't those calls, or at
least some of those calls, just take `pool' as well? (In other words,
why didn't you just leave the text "pool" for those calls, which after
your change to the top of the function would now refer to the caller
pool instead of the access baton pool?)

Thanks,
-Karl

> Modified: branches/issue-749-caching/subversion/libsvn_wc/entries.c
> ==============================================================================
> --- branches/issue-749-caching/subversion/libsvn_wc/entries.c (original)
> +++ branches/issue-749-caching/subversion/libsvn_wc/entries.c Mon Sep 23 20:22:24 2002
> @@ -1459,9 +1459,8 @@
> /* Ensure that NAME is valid. */
> if (name == NULL)
> name = SVN_WC_ENTRY_THIS_DIR;
> -
> - pool = svn_wc_adm_access_pool (adm_access);
> - name = apr_pstrdup (pool, name);
> + else
> + name = apr_pstrdup (svn_wc_adm_access_pool (adm_access), name);
>
> if (modify_flags & SVN_WC__ENTRY_MODIFY_SCHEDULE)
> {
> @@ -1473,7 +1472,8 @@
> /* If scheduling changes were made, we have a special routine to
> manage those modifications. */
> SVN_ERR (fold_scheduling (entries, name, &modify_flags,
> - &entry->schedule, pool));
> + &entry->schedule,
> + svn_wc_adm_access_pool (adm_access)));
>
> /* Special case: fold_state_changes() may have actually REMOVED
> the entry in question! If so, don't try to fold_entry, as
> @@ -1489,7 +1489,8 @@
> /* If the entry wasn't just removed from the entries hash, fold the
> changes into the entry. */
> if (! entry_was_deleted_p)
> - fold_entry (entries, name, modify_flags, entry, pool);
> + fold_entry (entries, name, modify_flags, entry,
> + svn_wc_adm_access_pool (adm_access));
>
> /* Sync changes to disk. */
> SVN_ERR (svn_wc__entries_write (entries, adm_access, pool));

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Sep 24 16:16:10 2002

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