Performance optimizations #9
Conversation
|
Any chance of before/after performance numbers? |
|
I've added some benchmarks and did further optimizations. |
|
I've redone my previous changes to match current state of lograge. Currently, I see an overall performance increase of ~60%. |
|
@splattael I'm gonna give this a good look over at the weekend and get it merged. Hold tight. |
|
@benlovell Thank you! I know, this PR got huge over time. I happily slim it down if you think I'm refactoring too much here. |
|
Hey @splattael, sorry for the delay. I've had a look over these changes and while they look good in principal - a good portion of the code has moved on since this was first proposed (unsurprisingly, of course.) If you're happy to rebase and deal with anything that surfaces, I'll be happy to get this landed in master. Thanks ❤️ |
We don't need to filter data because we skip them anyway if the key does not exist.
|
@benlovell I've redone my optmizations on current master. The results from benchmark after each commit starting with master:
(ips = instructions per second) Note, I had to work around Rubocop in some cases (e.g. splattael@63f61f7#diff-d3a7c7717409a1c1353805f9579b980bR88) Feedback is welcome |
|
Where did you have to work around rubocop and why? |
| exception, message = payload[:exception] | ||
| { status: 500, error: "#{exception}:#{message}" } | ||
| def extract_status(data, payload) | ||
| if (status = payload[:status]) |
There was a problem hiding this comment.
I am not a fan of assignments within the if conditions.
There was a problem hiding this comment.
For me, alternatives seem to be "uglier" here.
|
I like it, impressive performance improvement! 👍 |
| location = Thread.current[:lograge_location] | ||
| return unless location |
There was a problem hiding this comment.
@pxlpnk I had to work around rubocop here. I'd use a simple assignment within condition again.
|
This broke the tests. |
|
@pxlpnk Thanks for the heads-up! Fixed. |
|
👍 a classic 😴 commit |
|
@splattael this is looking awesome. If you update the changelog I'll get this merged. Thanks! 😍 |
|
@benlovell I've added an entry to the changelog. |
|
🌈 thanks! |
|
@benlovell Thanks for merging 💚 |
|
Persistence ftw 👍 |
Avoid object creation (e.g. Hash) as much as possible by passing `data`. Heavily inspired by roidrage#9
This PR tries to reduce method calls and object allocations.
See https://gist.github.com/splattael/5404417 for some benchmarks.