On 06.02.2015 09:54, Greg Stein wrote: > > > On Wed, Feb 4, 2015 at 3:35 AM, Branko Čibej <[email protected] > <mailto:[email protected]>> wrote: > > On 04.02.2015 03:22, Greg Stein wrote: > > On Mon, Feb 2, 2015 at 7:52 PM, <[email protected] > <mailto:[email protected]>> wrote: > >> ... > >> +++ subversion/branches/reuse-ra-session/BRANCH-README Tue Feb > 3 01:52:26 > >> 2015 > >> @@ -8,12 +8,19 @@ all changes made in the branch. > >> > >> STATUS > >> ====== > >> -+ Initial implementation > >> -- Do not call svn_ra_* methods in find_session_by_url() > because callback > >> - table may be destroyed at that time. > >> + > >> +done: > >> +- Initial implementation. > >> +- Separate active and inactive session lists. > >> + > >> +todo: > >> +- Fix timeout in davautocheck tests at > log_tests.py::log_diff_moved. > >> +- Limit the number of unused open sessions. > >> +- Run performance comparisons between trunk and branch to > prove that > >> + the RA session cache does in fact speed things up. > >> > > This is the part that I wonder about. If we had 1000 sessions, > then I > > *might* start to believe a separation would be interesting. > > > > A cache of sessions: sure; that's why we already had a cache. > > > > But to split that apart and keep multiple lists? Did you have an > indicator > > somewhere that this split could help? That "get me a connected > RA session" > > was somehow noticeably slow, relative to a simple iteration > sessions? > > The important reason is that with this split it's much easier to > manage > the cached idle sessions than it was before the change. Before, > all the > sessions were stored in an unordered hash table, and the difference > between 'idle' and 'active' was in a few bits of data in the cache > entry > struct. > > > "Code simplification" is a fine goal. But your log message said: > > - it speeds up the search for an inactive session to reuse, since > the search will no longer have to iterate over active sessions; > > So I imagine you can understand my wondering :-P > > > > Now, the idle sessions are stored in a doubly-linked list, which is > ordered. This will make it easier in future to limit the number of > cached idle sessions and to remove the least-recently-used ones > from the > cache. It also ensures that when we want to reuse a session, we'll > always pick the one that was released most recently, which reduces the > chance that the session might have become invalid (connection > timed out, > Kerberos token invalidated, etc.) while it's been idle. > > > Yups. > > > > And yes, I do expect that iterating through a shorter list will save > nanoseconds. :) > > > But of course! :-P
So I went and updated the log message. Should be less confusing, if more self-referential. :) -- Brane
