Thanks for the comments, Max. Replies below ...
On 06/05/2015 11:44 AM, Weijun Wang wrote:
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.
Yes - very good point, will change that.
SocketPermission:
JDK-431064? There should be 7 digits.
Oops, should be 4301064, will fix.
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?
Yeah maybe. An alternative is:
new CopyOnWriteArrayList<>(Collections.singletonList(up));
but it seems a bit like overkill.
The newly added tests are great, but are they already covered by JCK or
shall we contribute them to JCK?
I'll take a look, but this area is lacking in regression tests, so some
duplication I think is ok.
--Sean
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
