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

add Image.SubImageInto method #2903

Closed

Conversation

quasilyte
Copy link
Contributor

@quasilyte quasilyte commented Feb 4, 2024

Instead of allocating the result on heap for the user, allow the caller side to decide whether it should be heap-allocated or stack-allocated.

This new method allows us to have a zero-alloc SubImage routine for the use cases where we don't want to make an extra allocation.

The performance difference is measurable (comparison is done with benchstat):

name             old time/op    new time/op    delta
SubImage-12     203ns ± 2%      33ns ± 2%   -83.91%  (p=0.000 n=20+19)

name             old alloc/op   new alloc/op   delta
SubImage-12      112B ± 0%        0B       -100.00%  (p=0.000 n=20+20)

Using SubImage is ~203ns, SubImageInto is ~33ns. SubImage does 1 allocation, SubImageInto does none.

SubImage is rewritten in a way to avoid the code duplication. It should work identically to the previous contract (it returns nil if i is disposed, etc.)

Fixes #2902

What issue is this addressing?

#2902

What type of issue is this addressing?

Feature.

What this PR does | solves

It introduces a more efficient way of making subimages.

Instead of allocating the result on heap for the user,
allow the caller side to decide whether it should be heap-allocated
or stack-allocated.

This new method allows us to have a zero-alloc SubImage routine
for the use cases where we don't want to make an extra allocation.

The performance difference is measurable (comparison is done with benchstat):

	name             old time/op    new time/op    delta
	SubImage-12     203ns ± 2%      33ns ± 2%   -83.91%  (p=0.000 n=20+19)

	name             old alloc/op   new alloc/op   delta
	SubImage-12      112B ± 0%        0B       -100.00%  (p=0.000 n=20+20)

Using SubImage is `~203ns`, SubImageInto is `~33ns`.
SubImage does 1 allocation, SubImageInto does none.

SubImage is rewritten in a way to avoid the code duplication.
It should work identically to the previous contract (it returns nil
if `i` is disposed, etc.)

Fixes hajimehoshi#2902
Copy link
Owner

@hajimehoshi hajimehoshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SubImageInto breaks an assumption that an image's size never changes. So unfortunately I cannot accept this change.

We need more discussion.

@quasilyte quasilyte closed this Feb 4, 2024
@quasilyte quasilyte deleted the quasilyte_subimage_into branch February 4, 2024 19:29
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.

Alloc-free way of using SubImage
2 participants