search

softdevteam / lrpar

Rust LR parser
Other
1 stars 0 forks source link

Merge compatible nodes #56

Closed ltratt closed 6 years ago

ltratt commented 6 years ago

This PR merges together compatible nodes, helping to substantially reduce the search space. It gives at least a 2x speedup on pretty much everything I've tried it on. After a trivial renaming commit, the heart of this PR is https://github.com/softdevteam/lrpar/commit/f1305900dcc5f8f9270fb66aefae5c336a7eebcb which applies the merging idea to the Corchuelo recoverer -- and its commit message explains things in (I hope) plenty of detail. The third commit simply ports the same idea to the MF recoverer.

ptersilie commented 6 years ago

Just one typo in two places. Rest looks fine.

ltratt commented 6 years ago

Fixes pushed. If they look good to you, should I squash?

ptersilie commented 6 years ago

Looks like you squashed already. Shall I merge then?

ltratt commented 6 years ago

I haven't squashed yet. Just about to do so...

ltratt commented 6 years ago

Squashed.

ptersilie commented 6 years ago

Ah that's weird because I didn't see any new commit but the edits were shown in the last one. Anyway, looks good now.

ltratt commented 6 years ago

It was my fault: I pushed my updates to the wrong branch. The good news is that I did update things when I forced push, so everything was alright in the end. That said, it was still dumb of me. I will flagellate myself in a meagre attempt at recompense.

ptersilie commented 6 years ago

Haha! You would have gotten away with it. I thought it was just Github being weird.