webrev: http://cr.openjdk.java.net/~xuelei/7106773/webrev.03/
On 11/21/2011 4:34 PM, Weijun Wang wrote: > I removed most of the thread and keep some sections below: > > On 11/21/2011 03:30 PM, Xuelei Fan wrote: >> new webrev: http://cr.openjdk.java.net/~xuelei/7106773/webrev.02/ > > What is synchronized for? Why only on the for block? This won't prevent > map iteration and insertion at the same time. > You're right. The SynchronizedMap is not necessary. Changed to use normal hash map, and synchronizing the map in both iteration and insertion block. >> >>> >>> Another solution is to define a new interface: >>> package sun.security.util; >>> public interface Measurable { >>> public int getLength(); >>> } >>> >>> And updated P11Key and MSCPI Key accordingly to implement the >>> Measureable interface: >>> package sun.security.pkcs11; >>> ... >>> - abstract class P11Key implements Key, Measurable { >>> + abstract class P11Key implements Key { >>> ... >>> - int keyLength() { >>> + public int getLength() { >>> ... >>> } >>> >>> It is similar for MSCAPI Key. > > How about this approach? This looks very safe. > I also prefer this approach, although it need more updates in PKCS11 and MSCPI source code. If you vote for this approach, I will try to implement it. Thanks, Xuelei >>> >>>>>> >>>>>> Calling getDeclaredMethod each time is a waste. How about just >>>>>> >>>>>> while (clazz != null) { >>>>>> if (clazz.getName().equals(baseClassName)) { >>>>>> // do sth >>>>>> break; >>>>>> } >>>>>> clazz = clazz.getSuperclass(); >>>>>> } >>>>>> >>>>>> Also, you're using methods in reflection. Is it faster to use fields? >>>>>> >>>>> I also concerns about the override of the target method, although >>>>> there >>>>> is no such override at present. From some reports, it seems that >>>>> fields >>>>> reflection cost more cycles than method reflection. > > You mentioned "concerns about the override of the target method". > > Thanks > Max
