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

Ignore complexity of children in maps/slices/arrays #382

Merged

Conversation

klauspost
Copy link
Collaborator

When considering inlining maps/slices/arrays, apply a fixed cost and ignore the cost of the children.

If children are too expensive, they will not be inlined, but it shouldn't affect whether the map itself is inlined.

This only really applies when a map or slice type is aliased, otherwise it will not be considered for inlining.

Fixes #381

When considering inlining maps/slices/arrays, apply a fixed cost and ignore the cost of the children.

If children are too expensive, they will not be inlined, but it shouldn't affect whether the map itself is inlined.

This only really applies when a map or slice type is aliased, otherwise it will not be considered for inlining.

Fixes tinylib#381
@klauspost klauspost requested a review from philhofer December 8, 2024 17:46
@klauspost klauspost changed the title Ignore complexity of children in maps/slices Ignore complexity of children in maps/slices/arrays Dec 8, 2024
@philhofer
Copy link
Member

At some point it might be worth checking if the inlining actually does anything for performance at this point.

Back when I wrote this code in 2014 the Go compiler had awful inlining heuristics, and couldn't do "mid-stack inlining" at all. That's been fixed for a few years now.

@klauspost
Copy link
Collaborator Author

@philhofer To be honest, in my experience the in-liner is still quite bad. And a proper solution has been pushed down the road for years, with PGO promising magic improvements that are only useful in extremely limited scenarios.

I would love to be able to inline the Next call manually, so only the r.next() would be a function call.

Thanks for taking a look. I am pretty sure this will be fine performance-wise - and probably a net zero or minor gain.

@klauspost klauspost merged commit c0009df into tinylib:master Dec 9, 2024
4 checks passed
@klauspost klauspost deleted the ignore-map-slice-children-complexity branch December 9, 2024 10:42
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.

omitempty has no effect when node complexity exceeds inlining threshold
2 participants