Does FIXME: NYI in MutableCallSite.syncAll mean not yet implemented?

user_4062350 0
Viewed 47

In java.lang.invoke there are two interesting constructs, MutableCallSite and SwitchPoint. They are described as offering a way to change their targets (to an arbitrary different target for a MutableCallSite, or to a predefined when-invalidated target for a SwitchPoint), with other threads guaranteed to pick up the update, but with reduced overhead for the uses of the MCS or SP when it hasn't been changed (reduced, compared to a check with synchronization effects on every use). The SwitchPoint documentation says it can be built over MutableCallSite, and in the OpenJDK sources on GitHub, it is. So the place to look for the magic is in MutableCallSite.syncAll.

In the source for that method, the magic seems to have three steps:

  • Step 1 is a lazySet (JMM effects of setRelease) on a private AtomicInteger that no other thread will ever read;
  • Step 2 looks at all of the passed MutableCallSite references to confirm none are null;
  • Step 3 is a comment: // FIXME: NYI

To an untrained eye, there seems to be real work left for that comment to do. ;)

It seems like the developers of Android saw that comment and removed the method from Android because they didn't want to advertise an unimplemented method (even though MutableCallSite without syncAll doesn't seem like a useful construct). Were they right?

On the other hand, it seems really unlikely to be so glaringly missing its functionality. It has been in Java since Java 7, and apparently important projects like Nashorn and JRuby have made heavy use of MutableCallSite and SwitchPoint. I have not stumbled on any comments about them being broken because of it.

What's more, I understand that some implementations in the Java class library can look incomplete because parts of the magic are supplied by the JVM implementation. This seems like a prime example where that could be happening, but I haven't been able to find the details.

Does someone know the details? It would be good to know what the dependable behavior really is. The syncAll javadoc sounds very promising at first. Paraphrasing:

  1. its effect is to force all future readers of each call site's target to accept the most recently stored value
  2. it may (may??) block until all readers have (somehow) decached all previous versions
  3. reader threads may observe previous versions of the target until the syncAll call returns. (Does this mean they may not do so after the syncAll call returns? And if so, wouldn't that imply #2 is really something stronger than "may"?)
  4. it is "likely to be expensive" (as the trade-off for having getTarget be cheap; that's the magic part).

But later in the same javadoc, delving into JMM details, it says "If (some other thread) T executes a synchronization action A after the volatile write" ... then it must see the updated target.

That sounds a little ... less robust.

Is this all based on some JVM implementation knowledge that places an upper bound on how long that can take after the syncAll returns? If so, can that upper bound be stated?

If there really is nothing missing and the thing works as advertised, would it be worthwhile to remove the // FIXME: NYI in the source (and maybe replace it with an explanation of what really happens)?

For more: earlier version of this question posted on mlvm-dev list but didn't attract any answers.

jvm concurrency android java
1 Answers

The comment from boneill, and the linked concurrency-interest mailing list response from Remi Forax, seem to contain the answer. The MutableCallSite.syncAll method really does not do what is advertised, but the behavior ends up being adequate because MutableCallSite.setTarget does more than is advertised. So in the scheme

for ( i = 0; i < sites.length; ++ i )
  sites[i].setTarget(newTargets[i]);
MutableCallSite.syncAll(sites);

the necessary synchronization happens; it is just that most of the work happens before the syncAll. And likewise,

SwitchPoint.invalidateAll(switchpoints);

(which is implemented as a loop just like the above, touching the MutableCallSite underlying each SwitchPoint) also the necessary work happens.

All is not perfect with this partially-implemented scheme, though: moving the heavy lifting from syncAll into setTarget looks like it sacrifices the advertised benefit of paying the syncAll cost just once after updating n call site targets. In this implementation, that cost (mark_dependent_nmethods and deoptimize_all_marked by handshaking with all threads) happens n times, but each time only handling the dependencies of one CallSite. Likewise for SwitchPoint.invalidateAll with an array of n SwitchPoints.

I don't know why it's been left at this state of development since Java 7, but it looks like there is probably room for a nice performance boost if it ever gets implemented as originally documented.

user_40623500
Related