Hi Eric,
in Executable.java:
292 private Parameter[] privateGetParameters() {
293 if (null != parameters)
294 return parameters.get();
If/when SoftReference is cleared,privateGetParameters will be returning null
forever. Also Executable objects are already SoftReferenced from the Class. Do
we need another level of SoftReferencing?
279 public Parameter[] getParameters() {
280 // TODO: This may eventually need to be guarded by security
281 // mechanisms similar to those in Field, Method, etc.
282 Parameter[] raw = privateGetParameters();
283 Parameter[] out = new Parameter[raw.length];
284 // Need to copy the cached array to prevent users from messing
285 // with it
286 for (int i = 0; i < raw.length; i++) {
287 out[i] = new Parameter(raw[i]);
288 }
289 return out;
290 }
together with the copy constructor in Parameter.java:
48 Parameter(Parameter p) {
49 this.name = p.name;
50 this.modifiers = p.modifiers;
51 this.executable = p.executable;
52 this.index = p.index;
53 }
If I see right, then Parameter is an immutable object. You need not copy
the Parameter objects when copying the array. Just do a shallow copy
with Arrays.copy.
I assume native Executable.getParameters0() is constructing Parameter
objects with a reference to "this" Executable. Since one already has
access to "this" Executable (which is mutable), The Parameter objects
that hold a back reference are not exposing any shared mutable state.
And if they were, you would have to make a copy of Executable in the
copy-constructor of the Parameter - not just reference the original one.
I think there is some unnecessary copying and caching of annotation
arrays going on in Parameter.java.
You could base the annotations code on a cache that is already
established in the package-private
Executable.sharedGetParameterAnnotations() which returns a shared array
of arrays of parameter annotations.
Also why do you cache entire arrays for Parameter.getType() and
Parameter.getParameterizedType() when only a single element is needed?
Besides, there's no need to cache anything since there's already a cache
of that on the Executable level. You just need to expose it via
package-private abstract methods on Executable implemented in
Method/Constructor.
Regards, Peter
On 12/19/2012 11:43 PM, Eric McCorkle wrote:
Hello,
Please review the implementation of the core reflection API for method
parameter reflection. This changeset introduces a new class, Parameter,
which represents information about method parameters. It introduces a
new method into Executable which returns an array of the parameters for
the method or constructor represented by the Executable.
This is part of a larger set of changes which implement portions of the
method parameter reflection functionality in hotspot and javac.
The open webrev is here:
http://cr.openjdk.java.net/~jgish/~emccorkl/webrev/JDK-8004729/
The feature request is here:
http://bugs.sun.com/view_bug.do?bug_id=8004729
The latest version of the spec can be found here:
http://cr.openjdk.java.net/~abuckley/8misc.pdf
Thanks,
Eric
