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

Avoid unnecessary transform operations during prefetch #17013

Merged
merged 1 commit into from
Feb 1, 2025

Conversation

JkshirsagarMaxlinear
Copy link
Contributor

@JkshirsagarMaxlinear JkshirsagarMaxlinear commented Jan 30, 2025

This change will prevent prefetch to perform unnecessary transform operations on ARC buffer.

Motivation and Context

In current ZFS, the read performance with prefetch enabled was getting affected because of the extra and unnecessary operations happening on ARC buffer. While the flag to avoid the ARC buffer was set in prefetch path, it was not getting assigned when actual zio is getting data from Disk to ARC.
This change will reduce the CPU utilization and boost the performance.
Closes #17008

Description

This change is mainly focused on utilizing the existing flag to avoid the unnecessary operations (buffer allocation and transform operations) in prefetch.

How Has This Been Tested?

This change was tested on following system:

Type Version/Name
Distribution Name Ubuntu
Distribution Version 23.10
Kernel Version 6.5.0-44-generic
Architecture Intel(R) Xeon(R) Gold 6342 CPU
OpenZFS Version 2.2.6
Parameter Info
Benchmarking Tool IOR
Zpool Drives 6 NVME 1 TB Each
Zpool Configuration RAID0
Zpool Recordsize 128k
ZFS Primarycache all
ZFS compression on (lz4)
ZFS encryption off
ZFS checksum on
ZFS zfs_compressed_arc_enabled 1

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 amotin changed the title Closes #17008 Avoid unnecessary transform operations during prefetch Jan 30, 2025
@amotin
Copy link
Member

amotin commented Jan 30, 2025

@JkshirsagarMaxlinear I took a liberty to edit the PR title. Also I suspect that too long first line of the commit message may not pass the style check. It is recommended to have the first line to be shorter to be more meaningful in one line commit lists. Also the commit message is somewhat confusing, since this change is not specific to compression. It in general saves on buffer allocation and filling, that cost something even if the block is not compressed. Please amend and force-push to make it nice. Please read the documents referenced in the PR checklist if not yet.

@JkshirsagarMaxlinear
Copy link
Contributor Author

@amotin Thank you. I have updated the commit and pull request.

@amotin amotin added the Status: Code Review Needed Ready for review and testing label Jan 31, 2025
@JkshirsagarMaxlinear
Copy link
Contributor Author

Thank you @amotin. It says that I am not authorized to merge the pull request.

@amotin
Copy link
Member

amotin commented Jan 31, 2025

@JkshirsagarMaxlinear Only few people can merge here. I'll do it after CI tests pass.

@amotin
Copy link
Member

amotin commented Jan 31, 2025

@JkshirsagarMaxlinear But I see you added here unneeded merge commit. Please clean it up with rebase. There should be only one commit.

This change will prevent prefetch to perform unnecessary transform
operations on ARC buffer.

Signed-off-by: Jaydeep Kshirsagar <[email protected]>
@JkshirsagarMaxlinear
Copy link
Contributor Author

Hi @amotin
I cleaned the commit and PR. Can you please merge if everything seems fine.

@amotin amotin added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Feb 1, 2025
@amotin amotin merged commit 21205f6 into openzfs:master Feb 1, 2025
22 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enabling prefetch invokes unnecessary decompression calls affecting read performance
2 participants