Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed unneeded prefetch in IOFiber #3492

Merged
merged 2 commits into from
Mar 22, 2023

Conversation

djspiewak
Copy link
Member

This was an artifact of our -Xcheckinit days. The net result here is reducing the memory footprint of IOFiber by two full pointer widths, which is actually not a small amount. Surprisingly though, the results are almost exactly a wash for reasons I don't understand... except for parTraverse, which is mysteriously 20% faster now. I reproduced the performance delta across repeated runs and negative tests, so I think it's real, but I do find it fascinatingly bizarre.

Even more interestingly, I would have expected the performance results here to be essentially the inverse of #3471, which saw a statistically-significant degradation from shifting from a megamorphic invokevirtual to a getfield (at the cost of a slightly larger object map). The only explanation I have for why this PR has such minimal impact while that PR was so very noticeable (aside from experimental error) is that perhaps we're right up against an L1 page boundary on my test machine. If that's the case, rebasing #3471 on top of this PR and then re-running should produce a noticeable improvement.

@vasilmkd
Copy link
Member

I was planning to do this almost a year ago. I'm glad to see them go. I might have some more ideas for reducing the size of IOFiber even further.

As far as #3471, do be careful about increasing the memory footprint of each IO, it might be a noticeable change for some huge users.

vasilmkd
vasilmkd previously approved these changes Mar 12, 2023
@armanbilge armanbilge merged commit 1bbbc8b into typelevel:series/3.4.x Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants