Hi Alan,
Looking just at changes in jdk part...
On 06/14/2017 06:52 PM, Alan Bateman wrote:
It's time to bring the changes accumulated in the jake forest into
jdk9/dev. I'm hoping we are near the end of these updates and that we
can close the jake forest soon.
A summary of the changes that have accumulated for this update are
listed in this issue:
https://bugs.openjdk.java.net/browse/JDK-8181087
Aside from the important spec/javadoc updates, this update will bring
the -illegal-access option into JDK 9 to replace the
--permit-illegal-access option.
The webrevs with the changes are here:
http://cr.openjdk.java.net/~alanb/8181087/1/
-Alan
In j.l.Class, you have this new method:
2447 List<Method> getDeclaredPublicMethods(String name, Class<?>...
parameterTypes) {
2448 Method[] methods = privateGetDeclaredMethods(/* publicOnly
*/ true);
2449 List<Method> result = new ArrayList<>();
2450 for (Method method : methods) {
2451 if (method.getName().equals(name)
2452 && Arrays.equals(
2453 reflectionFactory.getExecutableSharedParameterTypes(method),
2454 parameterTypes)) {
2455 result.add(getReflectionFactory().copyMethod(method));
2456 }
2457 }
2458 return result;
2459 }
...where you use the 'reflectionFactory' field one time and
'getReflectionFactory()' method another time. The field might not
already be initialized...
3479 // Fetches the factory for reflective objects
3480 private static ReflectionFactory getReflectionFactory() {
3481 if (reflectionFactory == null) {
3482 reflectionFactory =
3483 java.security.AccessController.doPrivileged
3484 (new
ReflectionFactory.GetReflectionFactoryAction());
3485 }
3486 return reflectionFactory;
3487 }
3488 private static ReflectionFactory reflectionFactory;
Since 'reflectionFactory' field is not volatile, I would also introduce
a local variable into getReflectionFactory() so that the field is read
only once which would prevent theoretical possibility of reordering the
re-reading of the non-volatile filed before the (1st) reading of the
field which could cause getReflectionFactory() to return null.
In j.l.Module:
There are two places in the same method that contain exactly the same
fragment of code:
566 if (targets.contains(EVERYONE_MODULE) ||
targets.contains(other))
567 return true;
568 if (other != EVERYONE_MODULE
569 && !other.isNamed() &&
targets.contains(ALL_UNNAMED_MODULE))
570 return true;
Perhaps this could be factored out into separate private method which
could also be made a little more optimal (when other == EVERYONE_MODULE
and targets does not contain it, it is looked-up twice currently). For
example:
private static boolean isIncludedIn(Module module, Set<Module> targets) {
return
targets != null && (
targets.contains(EVERYONE_MODULE) ||
!module.isNamed() && targets.contains(ALL_UNNAMED_MODULE)
|| // ALL_UNNAMED_MODULE.isNamed() == false
module != EVERYONE_MODULE && module != ALL_UNNAMED_MODULE
&& targets.contains(module)
);
}
In j.u.ServiceLoader:
The following method:
601 /**
602 * Returns true if the given class is assignable to a class or
interface
603 * with the given name.
604 */
605 private boolean isAssignable(Class<?> clazz, String className) {
606 Class<?> c = clazz;
607 while (c != null) {
608 if (c.getName().equals(className)) {
609 return true;
610 }
611 for (Class<?> interf : c.getInterfaces()) {
612 if (interf.getName().equals(className)) {
613 return true;
614 }
615 }
616 c = c.getSuperclass();
617 }
618 return false;
619 }
...does not return true for situations like:
public interface A {}
public interface B extends A {}
public class C implements B {}
isAssignable(C.class, "A") ???
If A is a service interface and C is its implementation class, should C
be considered a valid service provider? If yes, the method might need
some stack or queue or recursion.
In IllegalAccessLogger:
Is the following method:
280 private void log(Class<?> caller, String what, Supplier<String>
msgSupplier) ...
Invoked frequently from different threads? Might synchronization in this
method be a performance bottleneck? There are some optimizations possible...
That's all for now.
Regards, Peter
