deep_enumerable icon indicating copy to clipboard operation
deep_enumerable copied to clipboard

Prefer yield without arguments to block.call where possible

Open nateberkopec opened this issue 9 years ago • 2 comments

Unfortunately, creating blocks in MRI is still very expensive, and you seem to pass around blocks quite a bit in this library. I could see this lib being used in some critical path/performance sensitive areas as well, so I think performance in this area is important.

I didn't take a deep enough dive in this to see where this would be possible (for example, this bit in deep_diff could probably use yield and block_given?). Otherwise, I think the library is extremely well written!

nateberkopec avatar May 11 '15 14:05 nateberkopec

Oof. I had no idea about the performance implications of block arguments. I think you're right that it makes sense to try to address this if possible. Since this bottleneck happens in such a nuanced way it's probably also worth putting in a few benchmark tests to make sure whatever fix is applied actually fixes the issue.

Thanks for spotting and reporting this!

dgopstein avatar May 11 '15 15:05 dgopstein

Agreed on tests. Minitest::Benchmark is a great little tool for this.

nateberkopec avatar May 11 '15 15:05 nateberkopec