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.



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()) { ...


Please correct me if I'm wrong: 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.

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;



Regards, Peter

Reply via email to