On 04.02.2015 03:22, Greg Stein wrote: > On Mon, Feb 2, 2015 at 7:52 PM, <br...@apache.org> 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. 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. And yes, I do expect that iterating through a shorter list will save nanoseconds. :) -- Brane