JEP 232 RFR: JDK-8065942 and JDK-8056179

[Sean Mullan]

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