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

[Anthony Scarpino]

I updated the webrev:
http://cr.openjdk.java.net/~ascarpino/8098580/webrev.01/

Comments below....

On 03/29/2016 11:08 AM, Valerie Peng wrote:
> Hi, Tony,
>
> <SessionManager.java>
> - line 243, 'be' should be "by"?

Yes, 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.

Yes, I believe you are correct, the -1 should be removed

>
> <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".

Sure.


> - line 1126 - 1127, it's clearer to have {} to clearly separate the code.

Oops.. Didn't notice it was missing..

> - 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.

Yes, you are right.


> - 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?

I put the check back in so that it will avoid doing a pkcs11 operations 
if the token isn't available.

> - The two dispose methods do entirely different things, it'd be better
> to name them differently.

Ok.. I changed the one that takes the session to disposeNative.

> 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
>>