Greg Stein wrote on Thu, Apr 26, 2012 at 14:01:24 -0400: > On Apr 26, 2012 1:56 PM, "Daniel Shahaf" <danie...@elego.de> wrote: > > > > Greg Stein wrote on Thu, Apr 26, 2012 at 13:29:55 -0400: > > > On Apr 26, 2012 12:55 PM, "Daniel Shahaf" <danie...@elego.de> wrote: > > > > > > > > danie...@apache.org wrote on Thu, Apr 26, 2012 at 16:30:45 -0000: > > > > > Author: danielsh > > > > > Date: Thu Apr 26 16:30:44 2012 > > > > > New Revision: 1330932 > > > > > > > > > > URL: http://svn.apache.org/viewvc?rev=1330932&view=rev > > > > > Log: > > > > > Follow-up to r1330906: fix/work-around trail reentrancy assertions > in > > > BDB. > > > > > > > > > > (Why is the uuid stored in svn_fs_t->fsap_data rather than in > svn_fs_t > > > proper?) > > > > > > > > > > * subversion/libsvn_fs/fs-loader.c > > > > > (cache_uuid): New function. > > > > > (svn_fs_open, svn_fs_create): Call it after everything else. > > > > > (svn_fs_open_berkeley, svn_fs_create_berkeley): Ditto. > > > > > > > > The purpose of this is to cause UUID to be cached in BDB's > > > > svn_fs_t->fsap_data so that the new-in-r1330906 calls to > > > > fs->vtable->get_uuid() wouldn't provoke the "run a transaction" > > > > codepath of svn_fs_base__get_uuid(). > > > > > > > > That seems a bit fragile; I'm not sure whether a better fix should be > > > > found (get the UUID within the existing trail) or whether this should > be > > > > documented and svn_fs_base__get_uuid() annotated to point out that the > > > > "run a trail" codepath now only runs once at the time of opening the > > > > svn_fs_t. > > > > > > > > Thoughts? > > > > > > Put the uuid into svn_fs_t and fill it at creation time using the > vtable. > > > Change svn_fs_get_uuid to return that, rather than using the vtable. > > > > > > > And delete the fs_vtable_t->get_uuid and fs_vtable_t->set_uuid callbacks > > altogether, I presume. > > Umm. No... don't you still need the vtable to access/set the persisted > value? >
No, I don't. svn_fs_set_uuid() can set a hypothetical svn_fs_t->uuid field itself; it only needs a vtable->set_uuid() helper if it wants to set fsap_data->uuid (which exists in both backends, but fsap_data is opaque to libsvn_fs). > I'm just suggesting that the vtable is used at init time, rather than every > call to get_uuid. > Where do you envision the UUID stored/cached --- in svn_fs_t or in svn_fs_t->fsap_data? I understand we move it from the latter to the former. > Cheers, > -g
