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

Feature/fixembedding #4176

Merged
merged 16 commits into from
Mar 13, 2023
Merged

Feature/fixembedding #4176

merged 16 commits into from
Mar 13, 2023

Conversation

jimidle
Copy link
Collaborator

@jimidle jimidle commented Mar 13, 2023

feat: Convert major structs to use embedded parent structs rather than allocating pointers

Many of the structs representing things like transitions, contexts, atn etc emulated Java inheritance by using pointers to instances of parent structs within their definitions. This doubles, triples and even quadruples the number of allocations needed to be made to build the recognizer structs and the prediction contexts.

This PR replaces the pointers with true embedding in the structs that are the hotspots within the runtime.

Seems like a big PR but in fact all the changes are doing the same thing.

jimidle and others added 16 commits March 10, 2023 13:52
… rather than allocate to pointers

Signed-off-by: Jim.Idle <jimi@idle.ws>
…uch as LexerATNConfig instead of by pointer

This is the first of many changes that switches the embedded class structure that was copying
Java class hieracrchy from allocations/new to proper embedding such that any struct is
allocated with one allocation not two or more. Main PR will cover what this means.

Signed-off-by: Jim.Idle <jimi@idle.ws>
… of pointer

Saves an extra allocation and helps the GC

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

Saves many allocations and grbage collections

Signed-off-by: Jim.Idle <jimi@idle.ws>
Should reduce allocation count by tons.

Signed-off-by: Jim.Idle <jimi@idle.ws>
…ntlr#4169)

* doc: Updates to some of the Go doc comments to start a ful ldocumentation cleanup

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

* doc: More documentation fixes.

Using this as a method of forcing myself to read every line of code in the runtime, and therefore
discover mistakes in the original implementation. And, of course, actually working docs for the
Go runtime, can only be a good thing.

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

* doc: More documentation fixes

Also changes the exporet level of a some variables and funcs that were not correct,
even though no user has currently needed them it would seem.

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

* doc: Many updates to document exported fuctions correctly and reformat the ingerited Java code

It looks like a massive amount of changes, but it is almost all doc or changing exports or renaming
unused paramters etc to make the Go linter happy.

No actual code changes yet.

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

* doc: More additions and corrections to the Go documentation for the runtime

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

* doc: Final clean of exported func and type documentation

There will be more to do here as there are a lot of things that are hidden internal to the antlr
package that probably should not be. There are also a lot of exported funcs and types without
any documentation, that will eventually need to be cleaned up.

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

* Changed Parser typings (antlr#4149)

Signed-off-by: Josua Frank <josua.frank@daimlertruck.com>
Co-authored-by: Josua Frank <josua.frank@daimlertruck.com>
Signed-off-by: Jim.Idle <jimi@idle.ws>

* fix: Fixes the failing go runtime test suite which was missing the /v4 off the replace option on the go.mod file (antlr#4163)

Arrrgh!

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

* present antlr before versioning (antlr#4156)

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

* fix: Prevent use of labels such as start= from generating code that clashes with builtin funcs (antlr#4161)

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

* Feature/gotestfix (antlr#4168)

* fix: Fixes the failing go runtime test suite which was missing the /v4 off the replace option on the go.mod file

Arrrgh!

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

* present antlr before versioning (antlr#4156)

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

* fix: Prevent use of labels such as start= from generating code that clashes with builtin funcs (antlr#4161)

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

* fix: Cater for the fact that some test rules use start as a label or rule name

As a fix for other cvode gen errors when start, end, or exception are used as
label names, they are now translated to have a suffix of `_` at code gen time.
However, the runtime tests sometimes use start as a rule name and so we must now
cater for this in the tests.

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

---------

Signed-off-by: Jim.Idle <jimi@idle.ws>
Co-authored-by: ericvergnaud <eric.vergnaud@wanadoo.fr>
Signed-off-by: Jim.Idle <jimi@idle.ws>

---------

Signed-off-by: Jim.Idle <jimi@idle.ws>
Signed-off-by: Josua Frank <josua.frank@daimlertruck.com>
Co-authored-by: Josua Frank <frank.josua@gmail.com>
Co-authored-by: Josua Frank <josua.frank@daimlertruck.com>
Co-authored-by: ericvergnaud <eric.vergnaud@wanadoo.fr>
…uch as LexerATNConfig instead of by pointer

This is the first of many changes that switches the embedded class structure that was copying
Java class hieracrchy from allocations/new to proper embedding such that any struct is
allocated with one allocation not two or more. Main PR will cover what this means.

Signed-off-by: Jim.Idle <jimi@idle.ws>
… of pointer

Saves an extra allocation and helps the GC

Signed-off-by: Jim.Idle <jimi@idle.ws>
Should reduce allocation count by tons.

Signed-off-by: Jim.Idle <jimi@idle.ws>
Signed-off-by: Jim.Idle <jimi@idle.ws>
Signed-off-by: Jim.Idle <jimi@idle.ws>
Rework of the variaous PredictionContexts has reduced memory allocations to between
30% and 50% of previous version.

Signed-off-by: Jim.Idle <jimi@idle.ws>
Signed-off-by: Jim.Idle <jimi@idle.ws>
@jimidle
Copy link
Collaborator Author

jimidle commented Mar 13, 2023

@parrt - Builds are still failing in the cpp artifact upload stage. The go builds are all good, so I think that this PR is good to go. It is a major improvement on memory allocations, with quite a bit more to come.

@parrt
Copy link
Member

parrt commented Mar 13, 2023

yay!

@parrt parrt merged commit 4a30b9a into antlr:dev Mar 13, 2023
@jimidle jimidle deleted the feature/fixembedding branch March 14, 2023 02:17
jimidle pushed a commit to jimidle/antlr4 that referenced this pull request Mar 28, 2023
* feat: Createa n Init routine for BaseATNConfig so we can embed sructs rather than allocate to pointers

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

* feat: Change BaseATNConfig to be properly embedded in other structs such as LexerATNConfig instead of by pointer

This is the first of many changes that switches the embedded class structure that was copying
Java class hieracrchy from allocations/new to proper embedding such that any struct is
allocated with one allocation not two or more. Main PR will cover what this means.

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

* feat: Change embedding for ATNBaseSimulator to true embedding instaed of pointer

Saves an extra allocation and helps the GC

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

* feat: Switch the use of pointers to embedded ATN states to true embeddding

Saves many allocations and grbage collections

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

* fix: Correct the way that PredictionContext is compared for merge

Should reduce allocation count by tons.

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

* Feature/docclean Greatly improve the godoc comments in the runtime (antlr#4169)

* doc: Updates to some of the Go doc comments to start a ful ldocumentation cleanup

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

* doc: More documentation fixes.

Using this as a method of forcing myself to read every line of code in the runtime, and therefore
discover mistakes in the original implementation. And, of course, actually working docs for the
Go runtime, can only be a good thing.

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

* doc: More documentation fixes

Also changes the exporet level of a some variables and funcs that were not correct,
even though no user has currently needed them it would seem.

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

* doc: Many updates to document exported fuctions correctly and reformat the ingerited Java code

It looks like a massive amount of changes, but it is almost all doc or changing exports or renaming
unused paramters etc to make the Go linter happy.

No actual code changes yet.

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

* doc: More additions and corrections to the Go documentation for the runtime

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

* doc: Final clean of exported func and type documentation

There will be more to do here as there are a lot of things that are hidden internal to the antlr
package that probably should not be. There are also a lot of exported funcs and types without
any documentation, that will eventually need to be cleaned up.

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

* Changed Parser typings (antlr#4149)

Signed-off-by: Josua Frank <josua.frank@daimlertruck.com>
Co-authored-by: Josua Frank <josua.frank@daimlertruck.com>
Signed-off-by: Jim.Idle <jimi@idle.ws>

* fix: Fixes the failing go runtime test suite which was missing the /v4 off the replace option on the go.mod file (antlr#4163)

Arrrgh!

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

* present antlr before versioning (antlr#4156)

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

* fix: Prevent use of labels such as start= from generating code that clashes with builtin funcs (antlr#4161)

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

* Feature/gotestfix (antlr#4168)

* fix: Fixes the failing go runtime test suite which was missing the /v4 off the replace option on the go.mod file

Arrrgh!

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

* present antlr before versioning (antlr#4156)

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

* fix: Prevent use of labels such as start= from generating code that clashes with builtin funcs (antlr#4161)

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

* fix: Cater for the fact that some test rules use start as a label or rule name

As a fix for other cvode gen errors when start, end, or exception are used as
label names, they are now translated to have a suffix of `_` at code gen time.
However, the runtime tests sometimes use start as a rule name and so we must now
cater for this in the tests.

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

---------

Signed-off-by: Jim.Idle <jimi@idle.ws>
Co-authored-by: ericvergnaud <eric.vergnaud@wanadoo.fr>
Signed-off-by: Jim.Idle <jimi@idle.ws>

---------

Signed-off-by: Jim.Idle <jimi@idle.ws>
Signed-off-by: Josua Frank <josua.frank@daimlertruck.com>
Co-authored-by: Josua Frank <frank.josua@gmail.com>
Co-authored-by: Josua Frank <josua.frank@daimlertruck.com>
Co-authored-by: ericvergnaud <eric.vergnaud@wanadoo.fr>

* feat: Change BaseATNConfig to be properly embedded in other structs such as LexerATNConfig instead of by pointer

This is the first of many changes that switches the embedded class structure that was copying
Java class hieracrchy from allocations/new to proper embedding such that any struct is
allocated with one allocation not two or more. Main PR will cover what this means.

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

* feat: Change embedding for ATNBaseSimulator to true embedding instaed of pointer

Saves an extra allocation and helps the GC

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

* fix: Correct the way that PredictionContext is compared for merge

Should reduce allocation count by tons.

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

* doc: Merge documentation updates

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

* feat: Rework predictions tructs to use emedding instead of pointers

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

* feat: more reworking of PredictionContext for embedding

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

* feat: Ensure that EmptyPredictionContext is correctly initialized

Rework of the variaous PredictionContexts has reduced memory allocations to between
30% and 50% of previous version.

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

* feat: Change from use of type casting to using stored type

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

* feat: Convert CommonToken to true emedding rather than pointers

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

---------

Signed-off-by: Jim.Idle <jimi@idle.ws>
Signed-off-by: Josua Frank <josua.frank@daimlertruck.com>
Co-authored-by: Josua Frank <frank.josua@gmail.com>
Co-authored-by: Josua Frank <josua.frank@daimlertruck.com>
Co-authored-by: ericvergnaud <eric.vergnaud@wanadoo.fr>
Signed-off-by: Jim.Idle <jimi@idle.ws>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants