Inline lambdas #423
Inline lambdas #423
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #423 +/- ##
==========================================
- Coverage 99.66% 97.89% -1.78%
==========================================
Files 14 14
Lines 603 618 +15
Branches 156 160 +4
==========================================
+ Hits 601 605 +4
- Misses 2 13 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@byroot Great work, thank you! |
I see, that's what I suspected, but I couldn't see any tested behavior, so I couldn't be sure. I'll see about deprecating them without losing performance. |
|
On that note, I don't know how important it is (I'm trying to optimize alba more for curiosity), but it offer quite a few of these "hooks", with multiple ways to do the same thing, and that makes a pretty big optimization I have in mind challenging. But I think I'll try it anyway once I get some time (probably next week). |
|
@byroot Right, so I'm looking for mentioning them in README with previous versions of Alba. |
|
I'm curious what else would be helpful to deprecate, perhaps we can soft-deprecate with a warning to see if any users are really impacted and figure out how to help them work around issues. I'm happy to help with that effort. It's lovely for Alba to have a lot of this flexibility, but at least I've found I don't need much beyond the basics. I'm so excited to see Alba getting this attention even if just an exercise in curiosity! Any speedups here will really help for my use case which is mostly reading and writing JSON via the newly improved JSON gem and Alba! 🙌 |
I couldn't find a reason for this pattern to call a method to get
a similar proc every time.
It does incur an extra method call, and a Proc object allocation.
In addition, in the case of `collection_converter`, the relatively
complex code can be replaced by a simpler and faster `map`.
Map is preferable over shifting objects into a new Array, because
it allows Ruby to directly allocate an Array of the right size
rather than to have to potentially resize it multiple times.
This isn't a big gain, but I think it makes the code easier to
read anyway.
Before:
```
Calculating -------------------------------------
alba 801.321 (± 0.6%) i/s (1.25 ms/i) - 4.080k in 5.091760s
Calculating -------------------------------------
alba 826.321k memsize ( 0.000 retained)
9.908k objects ( 0.000 retained)
6.000 strings ( 0.000 retained)
```
After:
```
Calculating -------------------------------------
alba 830.039 (± 0.8%) i/s (1.20 ms/i) - 4.182k in 5.038669s
Calculating -------------------------------------
alba 818.241k memsize ( 0.000 retained)
9.807k objects ( 0.000 retained)
6.000 strings ( 0.000 retained)
```
|
I pushed a new version that emits a warning if to define one of these two methods, and fallback to the old code. If you don't though, it continues to use the fast path.
I'm kinda planning (no promises) to prototype a branch that disregard backward compat to see how much it would be worth. |
|
Awesome, thank you! |
I couldn't find a reason for this pattern to call a method to get a similar proc every time.
It does incur an extra method call, and a Proc object allocation.
In addition, in the case of
collection_converter, the relatively complex code can be replaced by a simpler and fastermap.Map is preferable over shifting objects into a new Array, because it allows Ruby to directly allocate an Array of the right size rather than to have to potentially resize it multiple times.
This isn't a big gain, but I think it makes the code easier to read anyway.
Before:
After: