Hi Sean
Everything is fine, some comments:
FilePermission (also in SocketPermission, PropertyPermission,
ServicePermission):
int oldMask = ((FilePermission)existingVal).getMask();
int newMask = ((FilePermission)newVal).getMask();
if (oldMask != newMask) {
int effective = oldMask | newMask;
if (effective != oldMask) {
return new FilePermission(fp.getName(), effective);
}
}
return existingVal;
Is it worth checking "if (effective == newMask)"? Then you can return
newVal without creating a new FilePermission object.
SocketPermission:
JDK-431064? There should be 7 digits.
UnresolvedPermissionCollection:
The use of CopyOnWriteArrayList is cool, but add(up) right after an
empty one is created seems like an unnecessary copy-on-write. Maybe we
can file an RFE on CopyOnWriteArrayList itself?
The newly added tests are great, but are they already covered by JCK or
shall we contribute them to JCK?
Thanks
Max
On 06/04/2015 04:38 AM, Sean Mullan wrote:
This is the third and fourth in a series of fixes for JEP 232 (Improve
Secure Application Performance) [1].
webrev:
http://cr.openjdk.java.net/~mullan/webrevs/8065942-8056179/webrev.00/
bugs: https://bugs.openjdk.java.net/browse/JDK-8065942 and
https://bugs.openjdk.java.net/browse/JDK-8056179
This fix changes the Permissions and PermissionCollection subclasses to
use concurrent collections, which significantly reduces contention when
multiple threads are performing security checks. The bugs needed to be
fixed together, because removing the synchronized blocks in Permissions
revealed that several of the underlying PermissionCollection
implementations were not thread-safe.
Several new unit tests were also added to test basic functionality of
these classes.
With these fixes, the throughput of the Permissions.implies method
improves from approximately 6x to 10x when more than one thread is
running. Each of the bugs contains a performance chart with more details.
Thanks,
Sean
[1] http://openjdk.java.net/jeps/232
