metaborg / nabl

Spoofax' Name Binding Language
Apache License 2.0
7 stars 12 forks source link

Replace Cases with enum Tag and switch statements #99

Open Apanatshka opened 2 years ago

Apanatshka commented 2 years ago

I had a look at the Cases pattern in the statix.solver project and tried replacing it with switch statements. This seems to work out alright, doesn't gives much more ugly code (some cases I'd argue the code even gets better), and should be somewhat faster I think. Can you try running your benchmark/profiling example on this version and see if it makes a difference in performance? This doesn't get rid of all the lambdas, but it should help I think/hope.

Apanatshka commented 2 years ago

Welp, the tests fail, so I probably made a mistake in refactoring one of these things. I'll ping when this passes the tests.

Apanatshka commented 2 years ago

Cool, the tests run now.

Apanatshka commented 2 years ago

@AZWN we can look at this together tomorrow if you like

AZWN commented 2 years ago

Looks good, and sounds like a plan! Tomorrow after lunch?

Apanatshka commented 2 years ago

Works for me. If you can find the time, please run a benchmark / profiling example beforehand so we can evaluate the performance implications as well as the code changes.

Apanatshka commented 2 years ago

So the good news is I was able to fairly easily change to using switch for ITerm for you, which should help to make your profiling info a bit more straightforward to read. The bad news is it still does nothing for performance. Still, I hope it helps you a bit with profiling at least...