egraphs-good / egglog

egraphs + datalog!
https://egraphs-good.github.io/egglog/
MIT License
459 stars 54 forks source link

Fix `eval_lit` performance bug #441

Closed Alex-Fischman closed 1 month ago

Alex-Fischman commented 1 month ago

This PR fixes the performance bug in eval_lit as noticed by Yihong in 439.

Alex-Fischman commented 1 month ago

439

yihozhang commented 1 month ago

Nice! How does the flamegraph now look like for eggcc-extraction?

Alex-Fischman commented 1 month ago

I think something's wrong with my flamegraphs, but I'm getting ~10s now compared to ~25s before.

main: before

this branch: after

saulshanabrook commented 1 month ago

We didn't get there in the meeting, but would there be interest in adding codspeed tracking? They support flamegraphs in CI and will also show the diff in the flamegraph between a PR and main, showing which parts improved and which parts were degraded. If there is, I am happy to add it.

Alex-Fischman commented 1 month ago

I'm keeping this PR open to test with codspeed, but I will close it afterwards because it's subsumed by #442 .

codspeed-hq[bot] commented 1 month ago

CodSpeed Performance Report

Merging #441 will improve performances by 17.21%

Comparing Alex-Fischman:eval-lit-performance (840308a) with main (43de12f)

Summary

⚡ 2 improvements ✅ 85 untouched benchmarks

Benchmarks breakdown

Benchmark main Alex-Fischman:eval-lit-performance Change
eggcc-extraction 5.5 s 4.7 s +17.21%
string_quotes 475.3 µs 431.6 µs +10.11%