Skip to content

wire: Pass pointers by interface to writeElement#3579

Merged
davecgh merged 1 commit intodecred:masterfrom
jrick:common
Dec 8, 2025
Merged

wire: Pass pointers by interface to writeElement#3579
davecgh merged 1 commit intodecred:masterfrom
jrick:common

Conversation

@jrick
Copy link
Copy Markdown
Member

@jrick jrick commented Dec 5, 2025

For at least the past 5 years, due to garbage collector design, Go has not optimized small non-pointer types to fit inside interface headers, and instead always uses a pointer to data, if the type is not already a pointer. This means that writeElement, which was written before this time to avoid unnecessary heap allocations, was now causing heap allocations for each special cased non-pointer type.

This commit rewrites writeElement to only special case pointers to types, and to pass all elements by pointer.

@davecgh davecgh added this to the 2.1.2 milestone Dec 5, 2025
Copy link
Copy Markdown
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

Nice change. Thanks to you and @dajohi both for noticing that.

The changes look good, but I'm going to run it over the weekend to get some testing on it before approving and merging though.

For at least the past 5 years, due to garbage collector design, Go has not
optimized small non-pointer types to fit inside interface headers, and instead
always uses a pointer to data, if the type is not already a pointer.  This
means that writeElement, which was written before this time to avoid
unnecessary heap allocations, was now causing heap allocations for each
special cased non-pointer type.

This commit rewrites writeElement to only special case pointers to types, and
to pass all elements by pointer.

Co-authored-by: David Hill <dhill@mindcry.org>
Copy link
Copy Markdown
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

I ran this over the weekend with some additional debug code to see if there were any stragglers. No additional cases were reported and everything ran great.

@davecgh davecgh merged commit 45726d5 into decred:master Dec 8, 2025
34 checks passed
@davecgh davecgh modified the milestones: 2.1.2, 2.2.0 Dec 8, 2025
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.

2 participants