Dear JSR 292 Observers and MLVM members,
Daniel Heidinga of the IBM VM team has sent some excellent comments on
the recent draft APIs. Some of you who are working with the Reference
Implementation or the JSR 292 Backport may find that his comments
ring a bell for you, or you may have a use case (or abuse case!) for
one of the features we are trying to decide on. Please weigh in!
-- John
Begin forwarded message:
From: Daniel Heidinga <daniel_heidi...@ca.ibm.com>
Date: July 8, 2009 1:39:36 PM PDT
To: jsr-292...@jcp.org
Subject: Re: [jsr-292-eg] updated RI code and API javadoc
Reply-To: jsr-292...@jcp.org
We've taken a look through the Javadoc and have some comments.
==== Overall Comments ====
- There are references to sun.dyn.* classes in the documentation which
need to be removed. All classes that are part of the spec must be in a
java.dyn package.
- We still oppose adding "throws Throwable/Exception" to all
invokedynamic instructions. We have yet to see a valid reason for this
presented on the mailing list.
- MethodHandle.Lookup and the new Modularity JSR - has any thought
been given to how these will interact?
- Links to blogs, etc need to be replaced with the actual content.
This will keep the spec and the content in sync.
==== CallSite ====
* Cannot be a subclass of sun.dyn.CallSiteImpl
* initialTarget() method missing from javadoc. We like the
initialTarget() method as it provides a "fallback" method for HCR and
invalidation.
* Contradiction regarding "state is never null" and "If the bootstrap
method does not assign a non-null value to the new call site's target
variable, the method initialTarget()"
* CallSite(j.l.Object caller, j.l.String, MethodType)
* caller parameter and return value of callerClass() do not match
* Two options:
1. Fix CallSite constructor so that caller parameter is a j.l.Class.
2. Take an "invalidationToken" rather than the j.l.Class caller as a
parameter.
Change callerClass() to j.l.Object invalidationToken() and use this
value in the invalidation logic when invalidating.
Change Linkage.invalidateCallerClass(j.l.Class callerClass) to
Linkage.invalidateToken(j.l.Object token).
No difference to the VM whether it is holding onto a class or an object.
* nameComponents() and name()
* both link to your blog. Any relevant information there needs to be
moved to the javadoc so it is kept in sync with the doc.
* callerClass()
* can't return a class if CallSite takes an Object
* name()
* blog link contents need to be defined in the JavaDoc.
* "dangerous" characters missing '<', '>', ';', '['. '<' & '>' because
its a method name. Better to be exact when the list is so short.
* nameComponents()
* blog link contents need to be defined in the JavaDoc.
* This feels a lot like an implementation detail.
* toString()
* Either document the format or don't override Object's
implementation. User's may parse the output so all VM's should return
the same format or nothing at all.
==== InvokeDynamic ====
* "a one-to-one association" - what about call site splitting?
==== JavaMethodHandle ====
* Please provide a use case for this. We still don't see a need for
this and don't want to see it as API unless there is a strong reason
to add it.
* spec'd in terms of syn.dyn.* classes - Needs to only refer to
java.dyn classes.
* Please document the constructors for this class as well as any other
public API.
==== Linkage ====
* registerBootstrapMethod(j.l.Class, j.l.String)
* Why is the j.l.Class parameter called "runtime" in this method and
"callerClass" in registerBootstrapMethod(j.l.Class, MethodHandle)?
* Do we really need three versions of registerBootstrapMethod? Lets
aim for a small, simple API with as few unnecessary helpers as possible.
* invalidateCallerClass()
* invalidateToken proposal as CallSite takes an Object, not a Class.
See notes on CallSite.
* invalidateAll()
* "It is unspecified whether call sites already known to the Java code
will continue to be associated with invokedynamic instructions. If any
call site is still so associated, its CallSite.getTarget() method is
guaranteed to return null the invalidation operation completes." -
This needs to be defined. Otherwise, there will be issues with
different VMs operating differently.
* invalidateCallerClass() & invalidateAll()
* why do these methods have a return value instead of void?
==== MethodHandle ====
* Need to remove "subclasses syn.dyn.MethodHandleImpl"
* "Every invoke method always throws Exception" - We want to see a
valid reason for this. Otherwise, strongly opposed!
* "ldc instruction which refers to a CONSTANT_Methodref or
CONSTANT_InterfaceMethodref" - This feels like a hack. See our LDC
proposal (sent 06/17/2009) for adding CONSTANT_MethodHandleref_info &
CONSTANT_MethodTyperef_info ConstantPool entries.
* Please format the examples to be more readable by adding blank lines.
==== MethodHandles ====
* Lots of new "throws Throwable" - Can you provide the reasoning for
this? Why Throwable not Exception? If this is due to the project coin
requests, please provide background info as we oppose the change.
* Any method using conversions from convertArguments() should also
throw WrongMethodTypeException if the conversions fail:
permuteArguments, spreadArguments, etc
* There seems to be a lot of new methods: catchException,
throwException, dynamicInvoker, filterArguments, invoke_5-9,
methodType*, varargsInvoker, publicLookup
* This seems to be a strong argument in favour of Fredrik's proposal -
we won't be able to anticipate all of the useful adaptors down the road.
* invoke_0(MethodHandle), invoke_1(MethodHandle, j.l.Object), ....,
invoke(MethodHandle, j.l.Object ...)
* Why not take a page from Smalltalk naming conventions and call these
methods
invoke(MethodHandle)
invoke(MethodHandle, j.l.Object)
and finally,
invokeWithArguments(MethodHandle, j.l.Object[])?
This seems much cleaner then having 10 invoke_# methods.
* This might be a good place to prune our API as well. Do we really
need 10 of these invoke_# methods?
* genericInvoker()
* Seems cleaner in this revision.
* It appears that the behaviour from genericInvoker() has been split
between genericInvoker() and varargsInvoker(). Is this correct?
* dynamicInvoker()
* This is only correct if Fredrik's proposal is ignored - otherwise,
the invoker in the "equivalent to the following code" should not be an
exactInvoker.
* convertArguments()
* The conversion rules appear much cleaner.
* permuteArguments()
* Should throw WrongMethodTypeException if conversions fail.
("Pairwise conversions are applied.... as with convertArguments")
* collectArguments()
* "collecting a series of trailing arguments as arguments to a pre-
processor method" - What "pre-processor method"? Can you either define
or remove the sentence?
* dropArguments()
* Some of the examples seem wrong - // xy and // yz specifically.
* // xy should be // yz
* // yz should be // xy
* catchException()
* Scary "(how? TBD)" regarding passing args to the handler.
* Are changes to the original args by the target reflected in the
values passed to the handler when the exception occurs? Or are they a
pristine copy of the args as sent to target MH?
* Idea is ok
* throwException()
* Seems pointless.
* Can easily be done without having a special adapter for it.
* Do you have a valid use case for this? Otherwise, we should remove it.
* methodType()
* Please remove these methods. They are needless helpers and lead to
API bloat.
==== MethodHandles.Lookup ====
* Still lots of TBD
* How does this play with the new modularity jsr?
* new methods: in
* in(j.l.Class)
* "guaranteed to have no more access privileges than the original."
Unclear - please clarify.
* Can you provide a use case?
* toString()
* Please document the format or remove. All JVM's should have the same
format.
* findVirtual()
* "BUG NOTE" RI implementation problem. Not general issue.
_______________________________________________
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
