Stefan Fuhrmann <[email protected]> writes: >> What would be the reason to create a subpool here? > > > Thanks for the review, Evgeny! > > In this particular case, there is actually a reason: Neither this function > now its caller owns the SCRATCH_POOL, hence can't clean it. This function > potentially allocates a large amount of memory before the caller continues > to recourse into the node-tree. The SUBPOOL shaves off quite a bit of the > peak memory usage. > > Documented in r1639426. > >> >> It looks like this "subpool pattern" is quite common in the code around, >> but is there any reason for this? What would happen if we dropped all the >> subpools / local_pools / file_pools? > > > The code is older than it looks (first committed as fsfs-reorg.c but existed > before that) and was simply not written with the two-pool paradigm in mind. > OTOH, memory usage had been critical for fsfs-reorg, so every function tried > to keep its dynamic usage as low as feasible. > >> >> # grep svn_pool_create subversion/libsvn_fs_fs/stats.c @ r1632945 >> >> apr_pool_t * file_pool = svn_pool_create(pool); >> apr_pool_t *iterpool = svn_pool_create(scratch_pool); >> apr_pool_t *subpool = svn_pool_create(scratch_pool); >> apr_pool_t *subpool = svn_pool_create(scratch_pool); >> apr_pool_t *local_pool = svn_pool_create(pool); >> apr_pool_t *iterpool = svn_pool_create(local_pool); >> apr_pool_t *local_pool = svn_pool_create(pool); >> apr_pool_t *iterpool = svn_pool_create(pool); >> apr_pool_t *localpool = svn_pool_create(pool); > > > As of r1639549, the whole stats code uses the two-pool paradigm and none of > the local pools exists anymore, except for the one in get_phys_change_count.
Thanks! The new code (without the sub- and local-pools) looks much clearer to me. Regards, Evgeny Kotkov
