On 11/15/19 1:32 AM, Peter Levart wrote:
Hi Mandy,
http://cr.openjdk.java.net/~mchung/jdk14/8233527/webrev.03/
In Lookup's findBoundCallerClass and checkSecurityManager the javadoc
is still talking about private access only although the checks are now
made against full privilege access.
Fixed. @return in MethodHandles::lookup is also fixed.
Some possible nano optimization / simplification suggestions. Take
each with a grain of salt...
public Class<?> defineClass(byte[] bytes) throws
IllegalAccessException {
if (allowedModes != TRUSTED && !hasFullPrivilegeAccess()) {
TRUSTED == -1, which has all bits set. Therefore allowedModes ==
TRUSTED implies hasFullPrivilegeAccess(). In other words,
!hasFullPrivilegeAccess() implies allowedModes != TRUSTED. Above
condition could be simplified to:
if (!hasFullPrivilegeAccess()) { ...
OK with me.
Please correct me if I'm wrong:
These are interesting and I don't have the answer to final fields in
Lookup class. Maybe someone may chime in here. Otherwise I will find out.
All final instance fields in java.lang.invoke package are treated as
though they were annotated with @Stable right? If that is the case,
then all these checks will be folded into a constant if Lookup
instance can be folded into a constant when JITed, since allowedModes
field is final. But anyway, less branches in code sometimes makes code
faster. In this respect, the following:
public boolean hasFullPrivilegeAccess() {
return (allowedModes & PRIVATE) != 0 && (allowedModes &
MODULE) != 0;
}
...could also be written as:
public boolean hasFullPrivilegeAccess() {
return (allowedModes & (PRIVATE | MODULE)) == (PRIVATE |
MODULE);
}
So it's just a matter of taste probably which one is nicer to read.
I did consider both and no strong preference. No objection to use the
latter.
Also in the following part:
void checkSecurityManager(Class<?> refc, MemberName m) {
SecurityManager smgr = System.getSecurityManager();
if (smgr == null) return;
if (allowedModes == TRUSTED) return;
...the allowedModes == TRUSTED may be folded into a constant, while
security manager check above can not. Perhaps JIT is smart enough to
reorder these two checks since they both have the same path (return),
but if it doesn't, reordering them in code might be more optimal:
void checkSecurityManager(Class<?> refc, MemberName m) {
if (allowedModes == TRUSTED) return;
SecurityManager smgr = System.getSecurityManager();
if (smgr == null) return;
Regarding the performance difference, I think moving TRUSTED check to
the top is fine as it clearly will skip SM check.
http://cr.openjdk.java.net/~mchung/jdk14/8233527/webrev.04
Mandy
