gelisam / klister

an implementation of stuck macros
BSD 3-Clause "New" or "Revised" License
134 stars 11 forks source link

First pass performance engineering #215

Closed doyougnu closed 1 year ago

doyougnu commented 1 year ago

Hey David and Sam,

I've made a few performance tweaks in the expander. The overall result is that the testsuite now runs in about 10 seconds instead of 38 seconds and implicit-conversion-test finishes in 1 second compared to 10 seconds.

I wrote a case study on it for my book here: https://input-output-hk.github.io/hs-opt-handbook.github.io/src/Case_Studies/klister.html

please review it at your leisure. @david-christiansen I tried to request a review on github but that unfortunately requires you to be part of the IOG organization, so I went ahead and merged it. I've re-opened the IT ticket to move the book.

Please do not merge this before I squash. I like to keep a clean commit history.

doyougnu commented 1 year ago

40d4fccd20e4e43123cb415b64c4b251239022c3 and 2e45103d571a88c9ceaf247a8b715d751c0f3f86 have regressed the performance gain:

All 124 tests passed (16.11s)
Test suite klister-tests: PASS
Test suite logged to:

vs

  All 124 tests passed (9.89s)
  Test suite klister-tests: PASS

in 85205d58ebe922b0c7e203960813972362fac5c9

gelisam commented 1 year ago

Okay, then my intuition for performance is very bad, ignore my suggestions!

doyougnu commented 1 year ago

Okay, then my intuition for performance is very bad, ignore my suggestions!

No the suggestions were good my execution was bad! I checked my OS's CPU scaling governor and realized it was in powersave because I was on battery! Here are the actual difference (on AC power of course):

All 124 tests passed (9.91s)
Test suite klister-tests: PASS
Test suite logged to:

so basically no difference between the two implementations.

david-christiansen commented 1 year ago

I think the CPP is fine, but it's important to instantiate the debug flag both ways in CI.

doyougnu commented 1 year ago

Alright, that should be it. The root problem behind the packaging was hedgehog.