RFR 8098580: drainRefQueueBounds() puts pressure on pool.size()

[Valerie Peng]

Hi, Tony,

<SessionManager.java>
- line 243, 'be' should be "by"?
- line 271, why use SESSION_MAX - 1 instead of just SESSION_MAX? The 
check on line 286 make sure that the queue will not be empty. Actually, 
the comment has "until only one is left", but the condition is ">1", so 
shouldn't it be "until 2 left"? This is not what u changed, just noticed 
it and wonder if I read the code wrong.

<P11Key.java>
- line 1114 and 1115, can we rename these 2 variables? The terms session 
and token are all over this SessionKeyRef class. Some are local, some 
are member fields. It'd make the code much more easier to follow if they 
are named something less common, say "sess" or "tok".
- line 1126 - 1127, it's clearer to have {} to clearly separate the code.
- After the while-true loop (line 1116 - 1134), don't you have to call 
token.releaseSession(session) again? Otherwise, the most recently 
allocated op session seems to not released back to the pool.
- There was a session.token.isValid() check in the older dispose() 
method but not in the new one. Shouldn't we still check before getting a 
new op session?
- The two dispose methods do entirely different things, it'd be better 
to name them differently.

Thanks,
Valerie

On 3/1/2016 2:29 PM, Anthony Scarpino wrote:
> Hi,
>
> I need a review of this change to SunPKCS11 where it's a bit smarter 
> in disposing of native library object by reusing the same pkcs11 
> session is possible.  Additionally changes to the session pool to fix 
> a performance bottleneck.
>
> http://cr.openjdk.java.net/~ascarpino/8098580/webrev/
>
> thanks
>
> Tony
>