On 06.02.2015 09:54, Greg Stein wrote:
>
>
> On Wed, Feb 4, 2015 at 3:35 AM, Branko Čibej <brane_at_wandisco.com
> <mailto:brane_at_wandisco.com>> wrote:
>
> On 04.02.2015 03:22, Greg Stein wrote:
> > On Mon, Feb 2, 2015 at 7:52 PM, <brane_at_apache.org
> <mailto:brane_at_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.
>
>
> "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
Received on 2015-02-06 11:05:51 CET