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.

Sign up for GitHub

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

Jump to bottom

Pack dmu_buf_impl_t by 16 bytes #16684

Merged
merged 1 commit into from
Oct 25, 2024
Merged

Pack dmu_buf_impl_t by 16 bytes #16684

merged 1 commit into from
Oct 25, 2024

Conversation

amotin
Copy link
Member

@amotin amotin commented Oct 24, 2024

On 64bit FreeBSD this reduces one from 296 to 280 bytes. On small block workloads dbufs may consume gigabytes of ARC, and this saves 5% of it.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@amotin
Pack dmu_buf_impl_t by 16 bytes
39b3053 
On 64bit FreeBSD this reduces one from 296 to 280 bytes.  On small
block workloads dbufs may consume gigabytes of ARC, and this saves
5% of it.

Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
mcmilk
mcmilk approved these changes Oct 24, 2024
View reviewed changes
Copy link
Contributor

@mcmilk mcmilk left a comment

Choose a reason for hiding this comment

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

LGTM

behlendorf
behlendorf approved these changes Oct 24, 2024
View reviewed changes
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Nice. We should see the same saving on the Linux side as well.

@behlendorf behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Oct 24, 2024
@amotin
Copy link
Member Author

amotin commented Oct 24, 2024

We should see the same saving on the Linux side as well.

Right. I guess from 384 to 368 bytes. Locking primitives on Linux were bigger last time I checked.

@behlendorf behlendorf merged commit 94a03dd into openzfs:master Oct 25, 2024
19 of 20 checks passed
@amotin amotin deleted the dbuf_pack branch October 25, 2024 17:28
@amotin amotin mentioned this pull request Oct 31, 2024
Open
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Nov 1, 2024
@amotin @behlendorf
Pack dmu_buf_impl_t by 16 bytes
7e3ce4e 
On 64bit FreeBSD this reduces one from 296 to 280 bytes.  On small
block workloads dbufs may consume gigabytes of ARC, and this saves
5% of it.

Reviewed-by: Tino Reichardt <[email protected]>
Reviewed-by: Brian Atkinson <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Closes openzfs#16684
ixhamza pushed a commit to truenas/zfs that referenced this pull request Nov 11, 2024
@amotin @ixhamza
Pack dmu_buf_impl_t by 16 bytes
e303a29 
On 64bit FreeBSD this reduces one from 296 to 280 bytes.  On small
block workloads dbufs may consume gigabytes of ARC, and this saves
5% of it.

Reviewed-by: Tino Reichardt <[email protected]>
Reviewed-by: Brian Atkinson <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Closes openzfs#16684
arter97 pushed a commit to arter97/zfs that referenced this pull request Dec 9, 2024
@amotin @tonyhutter
Pack dmu_buf_impl_t by 16 bytes
1f5e1b9 
On 64bit FreeBSD this reduces one from 296 to 280 bytes.  On small
block workloads dbufs may consume gigabytes of ARC, and this saves
5% of it.

Reviewed-by: Tino Reichardt <[email protected]>
Reviewed-by: Brian Atkinson <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Closes openzfs#16684
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bwatkinson bwatkinson bwatkinson approved these changes

@mcmilk mcmilk mcmilk approved these changes

@behlendorf behlendorf behlendorf approved these changes

Assignees
No one assigned
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@amotin @behlendorf @mcmilk @bwatkinson