On 21/12/2009 10:32, Konstantin Kolinko wrote:
> 1)
> * quickly find that the session is already in sessions, use it and
> * carry on.
>
> This is only true if loading succeeds.
> If loading fails, nothing will be found in sessions, and loading will
> be retried. Not good. (Not fatal, but waste of time).
I'm not to concerned about this edge case since if the threads are far
enough apart the code will attempt multiple loads anyway. If we were
going to address this (and I'm not convinced we should) a cache of the
last 1000 sessions requested but not found would probably be a better
way to go.
> 2)
> if (sessionSwapInLocks.containsKey(id)) {
> swapInLock = sessionSwapInLocks.get(id);
> can be replaced with
> swapInLock = sessionSwapInLocks.get(id);
> if (swapInLock == null) { ...
Happy to change that.
> 3) I agree with sebb's proposal to make the field final.
And that.
>
> 3) Modified lines were indented with tabs, instead of spaces.
Sorry about that. I rebuilt my laptop and forgot to configure Eclipse to
use spaces. I'll fix that.
> Misprints:
> s/trues/tries/
> s/will re-creates/will re-create/
And those.
I'll make those changes and update the proposal.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
