Introduce interpreter V2 infrastructure. by aardvark179 · Pull Request #2438 · mozilla/rhino

aardvark179 · 2026-06-22T20:04:17Z

This is the first chunk of Interpreter V2 work. It introduces the call frame, interpreter data, instructions, operand and other associated classes, but I cut the actual interpreter out of this PR to make it a more manageable size, that will come in the next chunk.

The InstructionSimplification and KnownType classes allow the compiler to replace general instructions such as Add with specialised versions when the typee are known.

I've extracted this from #2426 with some additional tidy up.

gbrail

Thanks -- I have a bunch of questions but most are stylistic. No way to tell whether this is correct yet ;-)

I COULD suggest a few of the abstractions here like the line number table are great candidates for unit tests and if I were working on this I would insist on it (and let my AI get me started)

Finally, I think we should either bite the bullet and start using something like SLF4J in here like every other Java project on earth, or stick with "System.out" logging like other parts of Rhino.

gbrail · 2026-06-23T00:45:30Z

+import org.mozilla.javascript.interpreterv2.InstructionSimplification;
+import org.mozilla.javascript.interpreterv2.KnownType;
+
+public abstract class Instruction {


Is there a good reason to make this an abstract class rather than an interface with some default methods? I'm not sure there's a big difference but an interface might unlock some future flexibility.

Yes. Invoke virtual has a measurable performance difference from invoke interface because even if the receiver is not known the offset of the method is. This makes for an almost 10% improvement in some benchmarks.

gbrail · 2026-06-23T00:47:01Z

+import org.mozilla.javascript.interpreterv2.InstructionSimplification;
+import org.mozilla.javascript.interpreterv2.KnownType;
+
+public abstract class Operand {


In general I'd love a little bit of Javadoc on some of these generic abstractions that are going to serve us for years in the future. (Which is funny because I've noticed that AI loves to put comments on everything when I DON'T want them!) If people are going to use them to implement new things in the future they or their AI helpers might need some guidance.

I get why this one is abstract and not an interface.

Totally reasonable, I'll add some java docs.

gbrail · 2026-06-23T00:48:51Z

+
+        // Function header with metadata
+        String functionName = desc.name != null ? desc.name : "<anonymous>";
+        out.printf(


I think I see what's going on here. But in general do we want to introduce java.util.Logging now or stickwith our existing "System.out" logging approach? I feel like we should pick one or the other.

I agree that this should simply be the same style we use everywhere else. I think this was done here to capture stuff to our logs during development.

Okay, specialising this in the operands makes a small positive difference. I'll get those changes rolled into the appropriate commits.

gbrail · 2026-06-23T00:50:24Z

+
+    static boolean shouldDumpInstructions = RhinoConfig.get("rhino.printICodeV2", false);
+
+    public static final int EXCEPTION_TRY_START_SLOT = 0;


Should this be an enum?

Also we've had trouble with a few of these numeric constants that grow forever and we have started to use a convention like:

int FOO = 1;
int BAR = FOO + 1;
int BAZ = BAR + 1;

The reason is that when we add constants and merge conflicting PRs all the time then the renumbering gets out of hand. Not a big deal here but something to keep in mind for future PRs.

There's quite a few things I'd like to turn into enums. I'd thought I'd do this as part of an overall change to clean up our exception handling, but since this is really a duplicate of similar constants in Interpreter, it might be worth trying to do some of that now.

gbrail · 2026-06-23T00:52:30Z

+public class LineNumberTable {
+    private static final Logger LOG = Logger.getLogger(LineNumberTable.class.getName());
+
+    private final short[] pcs;


Are we sure that each set of offsets will never grow beyond 64k? (And I can help find a few corpuses of JavaScript to test if you'd like.) Or what happens if they do?

If this thing is one per interpreter I'd suggest that there's no advantage to using short here versus int.

Large version and test introduced.

gbrail · 2026-06-23T00:53:15Z

+
+            int lineIdx = 0;
+            for (int i = 0; i < numPcs; i++) {
+                pcs[i] = (short) (pcsList.get(i) & 0xFFFF);


Are we trying to avoid overflow here or something with all the bitmasking?

gbrail · 2026-06-23T00:55:38Z

+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.logging.Logger;


This will be the first use of java.util.logging in Rhino. Is it worth it? Or should we stick with System.out like in other parts of the codebase?

Either way, I don't recall in this PR how we're enabling or disabling logging but elsewhere we have used out existing "config" mechanism for this and I think we should use that here too.

aardvark179 · 2026-06-23T07:53:39Z

There should definitely be a unit test for the line number table, and there's one on our fork, but it looks like it's way later on my branch. Let me see about pulling that back to here.

Please do bring up any stylistic stuff. I've been doing some tidy as I've gone, but there's definitely room for more cleanup. Some things I want to tidy up, but want to wait to do properly, e.g. exception handling can definitely be unified and made simpler, but I'd like to do that after we've got v2 in because that will affect v1, v2, and class compilation.

aardvark179 · 2026-06-23T11:42:20Z

I've done some more clean up on this, and on #2426, still need to put some more tests on there at the right points.

aardvark179 · 2026-06-23T11:56:08Z

@gbrail I've sent you a message on Slack.

aardvark179 · 2026-06-23T16:32:49Z

I've pushed a large line number table implementation that can be used if we see anything that doesn't fit in a short during the build process. @andreabergia I'd appreciate your review on this as well.

Co-authored-by: Jimmy Miller <jimmy.miller@servicenow.com> Co-authored-by: Cam Walter <cam.n.walter@gmail.com>

andreabergia · 2026-06-26T10:03:25Z

+ * <p>Lookup uses binary search on pcs[] with O(log n) performance. Memory usage is approximately 12
+ * bytes per PC + 4 bytes per line number (no object overhead).
+ */
+public abstract class LineNumberTable {


All of this looks reasonable to me. There's a bit of duplication, but I can't think of any way to solve this better sadly.

andreabergia · 2026-06-26T10:03:50Z

+import java.util.List;
+import org.junit.jupiter.api.Test;
+
+class LineNumberTableTest {


This needs to be evolved to support both the short and int line number table versions.

gbrail · 2026-06-28T21:37:33Z

This looks OK to me now -- I'd still like to make sure we're clear going forward on enums versus integers and on Javadoc, but I'm also OK doing that later.

gbrail · 2026-06-28T22:21:56Z

Let's merge this now and keep going. Thanks!

aardvark179 force-pushed the aardvark179-interpreterv2-infra-1 branch 2 times, most recently from ff19dc2 to f609c4b Compare June 22, 2026 21:38

gbrail reviewed Jun 23, 2026

View reviewed changes

aardvark179 force-pushed the aardvark179-interpreterv2-infra-1 branch from f609c4b to 5256354 Compare June 23, 2026 11:24

aardvark179 force-pushed the aardvark179-interpreterv2-infra-1 branch 2 times, most recently from 36b7eaf to 47beb79 Compare June 24, 2026 13:28

aardvark179 and others added 3 commits June 24, 2026 14:42

Introduce interpreter V2 callsite and compiler data classes.

5e2b4b8

Co-authored-by: Jimmy Miller <jimmy.miller@servicenow.com> Co-authored-by: Cam Walter <cam.n.walter@gmail.com>

Unit tests for line number table.

b563540

Add support for large line number tables.

f10ab31

aardvark179 force-pushed the aardvark179-interpreterv2-infra-1 branch from 47beb79 to f10ab31 Compare June 24, 2026 14:04

andreabergia reviewed Jun 26, 2026

View reviewed changes

gbrail merged commit 0de2a42 into mozilla:master Jun 28, 2026
12 checks passed


		static boolean shouldDumpInstructions = RhinoConfig.get("rhino.printICodeV2", false);

		public static final int EXCEPTION_TRY_START_SLOT = 0;

Uh oh!

Conversation

aardvark179 commented Jun 22, 2026

Uh oh!

gbrail left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aardvark179 commented Jun 23, 2026

Uh oh!

aardvark179 commented Jun 23, 2026

Uh oh!

aardvark179 commented Jun 23, 2026

Uh oh!

aardvark179 commented Jun 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gbrail commented Jun 28, 2026

Uh oh!

gbrail commented Jun 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants