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

Better performance of bottomupNoBridges by avoiding the triggering of ClassCastException #2

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

datYori
Copy link

@datYori datYori commented Sep 12, 2024

from inkytonik#35

Hello.

We have been profiling our application, which utilizes Kiama, and have observed large amounts of ClassCastException errors when it processes fairly large trees. These errors originate from strategies with the following (common) shape:

everywhere(rule[Exp] { case BinaryExp(...) => ... ; case UnaryExp(...) => ... })

We understand the presence of ClassCastException is part of the design as it is explicitly handled in several places as an indication that a strategy didn't apply. Nevertheless, the performance of the tree processing code significantly improved after rewriting these strategies against the Any type, since this doesn't trigger a ClassCastException when passing through nodes that aren't Exp.

// now use Any
everywhere(rule[Any] { case BinaryExp(...) => ... ; ...})

In our current profiling info, the next couple of places where the exception is thrown are bottomupNoBridges and Cloner.deepclone.

If patching bottomupNoBridges like in this PR, there's again a significant speedup. I can provide some numbers, but if there's any benchmark you'd prefer us to run, please let us know.

Flagging a strategy with the Any type isn't a pattern one sees in Kiama's examples, but it appears to be an effective way to improve performance. We're curious to get your comments/feedback on that.

Many thanks.

@datYori datYori merged commit 18c24f1 into main Sep 12, 2024
1 check passed
@datYori datYori deleted the rawlabs-improvements branch September 12, 2024 08:06
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.

1 participant