metosin / sieppari

Small, fast, and complete interceptor library for Clojure/Script
Eclipse Public License 2.0
207 stars 21 forks source link

Performance got significantly slower #33

Closed zelark closed 4 years ago

zelark commented 4 years ago

Recently I noticed that it's not fast as it says. Here docs says it should be 64 µs for Pedestal sync, and 9 µs for Sieppari sync for a chain of 100 interceptors. First thing, in perf test there is only chain of 10 interceptors. Second, when I ran it, I was really surprised by the result I got. 11.87µs for Pedestal sync, and 824.35µs for Sieppari sync. You can see the whole result here. I have MacBook Air (mid 2013), which definitely isn't fast as Pro, but I don't expect so huge difference.

I started finding a point at where the issue came, and found #10. Along with other changes, it brought two ones after which performance got significantly slower. First one, which is most important, that now the check for async use satisfies? instead of just returning true or false. Because async? is used frequently it results in slowdown. Second one, which is also important it's that `Iterator' hint in leave phase has gone.

I fixed it, and got dramatically better result. You can find it here. It's just 1.63µs for sync case.

Should I create a PR?

@miikka @ikitommi

ikitommi commented 4 years ago

PR welcome. My bad for not running the perf tests after releasing the latest alpha.