Skip to content

Conversation

@cyqw
Copy link

@cyqw cyqw commented Jun 22, 2025

All antlr's target use same testcases in runtime-testsuite\resources\org\antlr\v4\test\runtime\descriptors.
provide rust runtime test runner for latest antlr.
#2

jimidle and others added 30 commits March 28, 2023 16:17
… flowcontrol - 50% performance improvement

  - Prior to this change, a recognition error was tracked by performing a panic(),
    which the generated code for rules would then use recover() to discover. However,
    recover() is not like catch(){} in Java and is expensive to run even if there is
    no panic() to find on the execution stack. Eliminating this and doing a simple check
    at the end of rule execution brings with it a massive performance improvement up to
    50% of CPU execution time. Now that collections and context caching is working correctly
    this is a significant improvement in execution time.

Signed-off-by: Jim.Idle <jimi@idle.ws>
Prior to this change, the runtime and generated code was using panic() and recover()
to perform error checking and reporting. This is extremely expensive and just not the
way to do it.

This change now uses goto, and explicit checking for error state after selected calls
into the runtime. This has greatly improved parser performance. Using the test code
provided by a recent performance issue report, the parse is now twoice as fast as the
issue raised was hoping for.

Signed-off-by: Jim.Idle <jimi@idle.ws>
* Add missing methods typing

Signed-off-by: hdpnguyen <hieu.dac.phuong.nguyen@mgm-tp.com>

* Fix PR comments

Signed-off-by: hdpnguyen <hieu.dac.phuong.nguyen@mgm-tp.com>

---------

Signed-off-by: hdpnguyen <hieu.dac.phuong.nguyen@mgm-tp.com>
Co-authored-by: hdpnguyen <hieu.dac.phuong.nguyen@mgm-tp.com>
Signed-off-by: Jim.Idle <jimi@idle.ws>
* do not modify String.prototype in js package

Signed-off-by: Jon Harris <harris.jb@gmail.com>

* remove notice file that is no longer relevant

Signed-off-by: Jon Harris <harris.jb@gmail.com>

---------

Signed-off-by: Jon Harris <harris.jb@gmail.com>
Signed-off-by: Jim.Idle <jimi@idle.ws>
Github action for upload was upgraded to v3 recently and the release is
unstable causing too many uploads to fail. Downgrading back to previous
version hasn't made significant improvement either.

Since the artifacts aren't exactly used by any chained job, failures for
uploading the artifact can be ignored. The artifacts are used mostly for
the purpose for debugging and so if needed the user can trigger specific
build again to get the artifact.

Signed-off-by: HS <hs@apotell.com>
Signed-off-by: Jim.Idle <jimi@idle.ws>
Signed-off-by: Jim.Idle <jimi@idle.ws>
Interfaces require two pointer lookups for functions and when used as
Generic, they require three. This change therefore yields a small
performance upgrade.

This change also corrects one or two copmarisons that were using pointer
comparison instead of Equals() and were bugs in the code I inherited.

Signed-off-by: Jim.Idle <jimi@idle.ws>
Signed-off-by: Jim.Idle <jimi@idle.ws>
Signed-off-by: Jim.Idle <jimi@idle.ws>
Signed-off-by: Jim.Idle <jimi@idle.ws>
… a single type and therefore a pointer

Signed-off-by: Jim.Idle <jimi@idle.ws>
Too much emulation of the Java runtime structure means that many structs
are being called via interfaces, which introduces and extra look up. When
you add that lookup to call via a pointer and then even an extra lookup for
generics, then it becomes significant. This seems like a big commit but
it is all just changing the one type declaration.

This now allows us to get rid of the stupid getters and setters that just
clutter the code and are not idiomatic of Go anyway.

Signed-off-by: Jim.Idle <jimi@idle.ws>
Go does not use Getters and Setters unless there is some very good reason such
as copy on read or write. They can stop the compiler from inlining things so they
are now gone from ANTConfigSet, whihc is no longer an interface.

Signed-off-by: Jim.Idle <jimi@idle.ws>
Like other struct implementations, this was hidden by an
interface that did not need to be there. the only difference between
a lexer and parser config is the Hash/Equals and extra methods that only
the lexer calls.

Signed-off-by: Jim.Idle <jimi@idle.ws>
Signed-off-by: Jim.Idle <jimi@idle.ws>
Signed-off-by: Jim.Idle <jimi@idle.ws>
…f DFA collections

Signed-off-by: Jim.Idle <jimi@idle.ws>
Also corrects the formatting of some of the files as this can create false
compares in git.

Signed-off-by: Jim.Idle <jimi@idle.ws>
…er look

Signed-off-by: Jim.Idle <jimi@idle.ws>
…in some cases

Signed-off-by: Jim.Idle <jimi@idle.ws>
…each run

Signed-off-by: Jim.Idle <jimi@idle.ws>
Another small gain, but for one good parser this goes from 90ms to 64ms (for 16 large files)
so it won't make a lot of difference for poor parser, but good ones will see a nice kick here

Signed-off-by: Jim.Idle <jimi@idle.ws>
  o Uses reflection to check that the tree structure is all correct
  o Disables test that uses superClass as this basically doesn't work/isn't a concept in Go.

Signed-off-by: Jim.Idle <jimi@idle.ws>
Signed-off-by: Ahmad Tameem <ahmad.tameem97@gmail.com>
Signed-off-by: Adrian Jutrowski <adrian.jutrowski@gmail.com>
…mpacted

Signed-off-by: Adrian Jutrowski <adrian.jutrowski@gmail.com>
Signed-off-by: cyqw <cyqw@163.com>
Signed-off-by: cyqw <cyqw@163.com>
Signed-off-by: cyqw <cyqw@163.com>
@alexsnaps
Copy link

@cyqw care to fix the conflict? Should be straight forward. I'll look into merging this PR this week!

@@ -22,7 +22,7 @@ fn main() {
None,
None,
];
let antlr_path = "../antlr4/tool/target/antlr4-4.8-2-SNAPSHOT-complete.jar";
let antlr_path = "../../../tool/target/antlr4-4.13.3-SNAPSHOT-complete.jar";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏 Thanks! Looks like I had these (relative dir) changes only local still 🤦

@alexsnaps
Copy link

@cyqw also, I'll check it all out locally, but I'm somewhat confused by the s/isize/i32/g 🤔

@cyqw
Copy link
Author

cyqw commented Oct 8, 2025

@cyqw also, I'll check it all out locally, but I'm somewhat confused by the s/isize/i32/g 🤔
The generated ATN data is is use i32 as data type (maybe better to use u32, but need change the antlr tool). All the value from ATN should be i32.
Input stream is also use i32 as data type for data index, It support file less than 2G, It can change to isize

@alexsnaps
Copy link

@cyqw Ah! Yes... all that java code that uses int (i.e. signed 32 bits, i32)... makes sense.

cyqw added 2 commits October 9, 2025 05:47
Signed-off-by: cyqw <cyqw@163.com>
Signed-off-by: cyqw <cyqw@163.com>
@cyqw
Copy link
Author

cyqw commented Oct 17, 2025

@alexsnaps
How about the review of this PR?

@alexsnaps
Copy link

I'm really sorry @cyqw - there were some a few advisories (as you may have inferred from the work that happened) raised in cel-rust that are now mitigated. But sadly recreates conflicts for this branch (now they shouldn't matter, and be straight resolves).

With that being said tho, if you get to resolve these and update this branch, then great, if you can't... I'll give it a shot later this week. But once that is done, I can try your changes out and hopefully merge this week.

Again, sorry about this, but you can imagine these issues were more important to be addressed asap.

@alexsnaps
Copy link

Also, I might have been unclear, but feel free to keep the alignment with java to i32, it makes sense afaict given your explanation.

…st-target

Signed-off-by: cyqw <cyqw@163.com>
@cyqw
Copy link
Author

cyqw commented Oct 21, 2025

I'm really sorry @cyqw - there were some a few advisories (as you may have inferred from the work that happened) raised in cel-rust that are now mitigated. But sadly recreates conflicts for this branch (now they shouldn't matter, and be straight resolves).

With that being said tho, if you get to resolve these and update this branch, then great, if you can't... I'll give it a shot later this week. But once that is done, I can try your changes out and hopefully merge this week.

Again, sorry about this, but you can imagine these issues were more important to be addressed asap.

Resolved the conflict

@alexsnaps
Copy link

alexsnaps commented Oct 21, 2025

What am I doing wrong here:

$ mvn -DskipTests package
<SNIP>
[INFO] Invoking controller method: org.antlr.v4.unicode.UnicodeDataTemplateController.getProperties()
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for ANTLR 4 4.13.3-SNAPSHOT:
[INFO] 
[INFO] ANTLR 4 ............................................ SUCCESS [  0.151 s]
[INFO] ANTLR 4 Runtime .................................... SUCCESS [  1.761 s]
[INFO] ANTLR 4 Tool ....................................... FAILURE [  0.640 s]
[INFO] ANTLR 4 Maven plugin ............................... SKIPPED
[INFO] ANTLR 4 Runtime Tests (4th generation) ............. SKIPPED
[INFO] ANTLR 4 Tool Tests ................................. SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  2.904 s
[INFO] Finished at: 2025-10-21T08:56:16-04:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal com.webguys:string-template-maven-plugin:1.1:render (default) on project antlr4: Unable to invoke controller: org.antlr.v4.unicode.UnicodeDataTemplateController (no such attribute: propertyCodePointRanges) -> [Help 1]              

Works fine (on my machine 🤦 ) on antlr/antlr4:dev tho

@cyqw
Copy link
Author

cyqw commented Oct 21, 2025

This plugin invoke a static method "getProperties" in UnicodeDataTemplateController.java
Check your local file

@alexsnaps
Copy link

alexsnaps commented Oct 21, 2025

Ok, well I get that... but, wait... is your PR here WIP or ... working?
What's the state of things here?
cause org.antlr.v4.unicode.UnicodeDataTemplateController is obviously a class of this repo... Are my expectations of keeping this project compiling wrong? I'm probably missing something here...

@alexsnaps
Copy link

Hmm... mvn clean got me to a different problem. At least a reproducible one.

But can you please let me know what you think this PR achieves?

@alexsnaps
Copy link

For some reason my environment was in bad state.
I could compile it all (-DskipTests) and test generated code against the generated parser for CEL and (what will be a new version of) the antlr4rust crate of this PR.

That looks really promising! I'll look into merging this if not today, tomorrow...

But again, what do you think is the state of this PR?
afaict mvn test fails...

@cyqw
Copy link
Author

cyqw commented Oct 21, 2025

For some reason my environment was in bad state. I could compile it all (-DskipTests) and test generated code against the generated parser for CEL and (what will be a new version of) the antlr4rust crate of this PR.

That looks really promising! I'll look into merging this if not today, tomorrow...

But again, what do you think is the state of this PR? afaict mvn test fails...

I think this PR is ready to merge.

@cyqw
Copy link
Author

cyqw commented Oct 21, 2025

This PR rebase with dev branch of antlr4, it use github actions to run CI on branch master dev. action
I think we need change main branch to dev.

@alexsnaps
Copy link

Merging this, let's add rust-target to the hosted.yml action until we're ready to open a PR against antlr's upstream

@alexsnaps alexsnaps merged commit 72d83d0 into antlr4rust:rust-target Oct 22, 2025
@alexsnaps
Copy link

See #16 & #17

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.