Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Begin to use invokedynamic in the bytecode #1645

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gbrail
Copy link
Collaborator

@gbrail gbrail commented Sep 24, 2024

This begins to use invokedynamic instructions in Rhino's bytecode.

It replaces the operations that call the ScriptRuntime operations for
common property operations like setting and getting properties and
elements, and common context operations like setting and getting variables,
with invokedynamic instructions.

It also adds components that will wire up those invokedynamic instructions
to the appropriate ScriptRuntime operations using the Dynalink package.

The result should be that Rhino behaves exactly the same and performs the
same as well.

However, once this is implemented we can begin to create additional dynalink
"linkers" that do specific things to optimize performance based on what is
happening at runtime.

Use Dynalink, but only install a dynamic linker for now
Also make the DefaultLinker code have a bit less copy and paste
This feature should never be invoked on Android.
@gbrail
Copy link
Collaborator Author

gbrail commented Sep 28, 2024

Unless there are no objections, I'd like to merge this. Just so that you all understand the scope:

Downsides:

  1. It adds a dependency on the jdk.dynalink module. That should not be a problem for anyone I can think of as long as we make sure that this code is never activated on an Android platform (and I think that it will be OK)
  2. It adds an extra layer of abstraction to the bytecode which requires understanding more stuff to understand what it does. I am going to experiment more with the structure of the code. I fear in the current state it will be hard to understand.

Upsides:

  1. The bytecode is smaller and IMO easier to read.
  2. I have an optimization waiting in the wings that will make constants faster. However it only works for "real constants," and sadly existing JavaScript benchmarks tend not to actually use "const." (I also tried a more aggressive optimization that assumed things were const until they aren't, but that needs more work.)
  3. I think that further optimizations are possible for the various math operations.
  4. I have spent lots and lots of time trying to make property and variable accesses more efficient, and so far nothing makes a significant difference, but I will keep working on it.

@p-bakker
Copy link
Collaborator

No objections here!

Wrt to the downsides: dunno how feasible it is, but some documentation on the bytecode generation may help? Got the idea that only a few people understand how that works right now. I for one don't at all 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants