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

[Valerie Peng]

Updated webrev looks fine~
Thanks,
Valerie

On 4/1/2016 10:11 AM, Anthony Scarpino wrote:
> 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
>>>
>