search

jhipster / prettier-java

Prettier Java Plugin
http://www.jhipster.tech/prettier-java/
Apache License 2.0
1.08k stars 103 forks source link

chore: upgrade to Chevrotain 11 #616

Closed jtkiesel closed 8 months ago

jtkiesel commented 11 months ago

What changed with this PR:

  1. Both java-parser and prettier-plugin-java are converted from CJS to ESM, to facilitate the upgrade to Chevrotain 11 (which is now ESM only).
  2. Chevrotain is upgraded to 11.0.3. This caused parsing to be roughly 100% slower due to some changes that were made affecting backtracking performance.
  3. Chevrotain Allstar is used to improve performance by greatly reducing backtracking. This caused parsing to be roughly 75% faster.

This PR's net performance change to parsing seems to be around 47% faster. It should be possible to achieve even further performance gains by removing the few remaining instances of backtracking. However, doing so appeared to be nontrivial, so for now I stopped after removing the ones that gave us easy wins.

Relative issues or prs:

Closes #423 Obsoletes #486

jtkiesel commented 10 months ago

Rebased and resolved merge conflicts.

clementdessoude commented 10 months ago

If @bd82 or @msujew could have a look, it would be great, as they are way more experts on chevrotain's parsers' internals than I am !! If they cannot, we can merge it next week, maybe.

I propose to leave this out in the next release (which would include #611 and #614), so that we can release it today

jtkiesel commented 10 months ago

@clementdessoude Thanks for the review! I agree with waiting to merge this until after the next release -- wouldn't want to hold up those Java 21 feature additions for this upgrade. I just rebased (to resolve merge conflicts with main following the merge of #615) and applied the 2 changes we discussed. Will wait to see if Shahar and/or Mark have an opportunity to review.

clementdessoude commented 10 months ago

Sorry @jtkiesel, it seems that the merge of the other PR created some conflicts on this one :/

jtkiesel commented 10 months ago

@clementdessoude No worries! Rebased and resolved conflicts. 👍

bd82 commented 9 months ago

@msujew fyi - usage of https://github.com/TypeFox/chevrotain-allstar

jtkiesel commented 8 months ago

@clementdessoude Apologies for the mention, but can you give this another look when you have a minute? Should be good to merge.

clementdessoude commented 8 months ago

@jtkiesel so sorry for the delay, and thanks for the mention (and once again for the great work ;))