On Wed, Feb 4, 2015 at 3:35 AM, Branko Čibej <[email protected]> wrote:
> On 04.02.2015 03:22, Greg Stein wrote: > > On Mon, Feb 2, 2015 at 7:52 PM, <[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 Thx, -g
