Skip to content

Use a simple DFA to parse operators.#28

Open
JaredDavis22 wants to merge 2 commits into
scijava:mainfrom
JaredDavis22:alternativeoperatorparsing
Open

Use a simple DFA to parse operators.#28
JaredDavis22 wants to merge 2 commits into
scijava:mainfrom
JaredDavis22:alternativeoperatorparsing

Conversation

@JaredDavis22

Copy link
Copy Markdown
Contributor

Stores a copy of the starting node in ParseOperation.

@ctrueden

ctrueden commented Jul 17, 2026

Copy link
Copy Markdown
Member

This adds 133 net lines of code, and IMHO makes the parseOperator method harder to understand. What are the benefits of this change?

@ctrueden

Copy link
Copy Markdown
Member

See #24 (comment) for related discussion.

@ctrueden
ctrueden force-pushed the alternativeoperatorparsing branch from cff73b0 to 09f01c5 Compare July 17, 2026 21:28
@ctrueden

Copy link
Copy Markdown
Member

FYI, I rebased this over the latest main branch. (No conflicts.)

@JaredDavis22

JaredDavis22 commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

The majority of my operators are * = + and -. The existing code, which uses a loop through a list, has these near the end of a 50+ element list. So, every potential operator has to go through 40+ dead ends (even operators which are not implemented) before a hit is found. The dfa structure finds an operator very quickly and also rejects non operators very quickly. It seems it is worth the code complexity for the better performance.

This link should show the dfa visually from a dot file.

@JaredDavis22

Copy link
Copy Markdown
Contributor Author

The graph code and some slight reformatting is in https://github.com/JaredDavis22/parsingtonFork/tree/matcherUsingThreadLocalguess

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