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

isa-l: update isa-l to v2.18 #15895

Merged
merged 1 commit into from Jul 7, 2017

Conversation

Projects
None yet
4 participants
@ganeshmaharaj
Contributor

ganeshmaharaj commented Jun 23, 2017

This upgrade brings

  • Complete rewrite of DEFLATE optimizations resulting in 5X better
    throughput and compression ratios comapred to zlib, lz4, lzo and 2X
    better decompression performance when compared to zlib.
  • AVX512 improvements to multi-buffer versions of MD5, SHA-1 and SHA-256
    cryptographic hashing functions resulting in 3X better in performance
    compared to the AVX2 generation.

This update has the potential to improve bluestore compression
and dedup performance.

Signed-off-by: Ganesh Mahalingam ganesh.mahalingam@intel.com
Signed-off-by: Tushar Gohad tushar.gohad@intel.com

@liewegas liewegas changed the title from WIP: Update isa-l to v2.18 to isa-l: update isa-l to v2.18 Jun 26, 2017

@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas Jun 26, 2017

Member
/home/jenkins-build/build/workspace/ceph-pull-requests/src/compressor/zlib/ZlibCompressor.cc:19:37: fatal error: isa-l/include/igzip_lib.h: No such file or directory
Member

liewegas commented Jun 26, 2017

/home/jenkins-build/build/workspace/ceph-pull-requests/src/compressor/zlib/ZlibCompressor.cc:19:37: fatal error: isa-l/include/igzip_lib.h: No such file or directory
@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas Jun 26, 2017

Member

retest this please

Member

liewegas commented Jun 26, 2017

retest this please

@liewegas liewegas added the needs-qa label Jun 29, 2017

@tsg-

This comment has been minimized.

Show comment
Hide comment
@tsg-

tsg- Jun 29, 2017

@liewegas following up on comments during the community perf call this morning .. we'll share data supporting perf gain claims related to AVX512 and deflate-rewrite. Thanks.

tsg- commented Jun 29, 2017

@liewegas following up on comments during the community perf call this morning .. we'll share data supporting perf gain claims related to AVX512 and deflate-rewrite. Thanks.

@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas Jun 29, 2017

Member
Member

liewegas commented Jun 29, 2017

@liewegas

This comment has been minimized.

Show comment
Hide comment
@ganeshmaharaj

This comment has been minimized.

Show comment
Hide comment
@ganeshmaharaj

ganeshmaharaj Jul 5, 2017

Contributor

@liewegas please find the comparison numbers between the current version of ISA-L (2.16) and the proposed one (2.18) here. The compression latency improvement from bluestore FIO test ranges between 16% and 37%.

Contributor

ganeshmaharaj commented Jul 5, 2017

@liewegas please find the comparison numbers between the current version of ISA-L (2.16) and the proposed one (2.18) here. The compression latency improvement from bluestore FIO test ranges between 16% and 37%.

@tchaikov

This comment has been minimized.

Show comment
Hide comment
@tchaikov

tchaikov Jul 6, 2017

Contributor

retest this please.

Contributor

tchaikov commented Jul 6, 2017

retest this please.

Update isa-l to v2.18
This upgrade brings
-  Complete rewrite of DEFLATE optimizations resulting in 5X better
   throughput and compression ratios comapred to  zlib, lz4, lzo and 2X
   better decompression performance when compared to zlib.
-  AVX512 improvements to multi-buffer versions of MD5, SHA-1 and SHA-256
   cryptographic hashing functions resulting in 3X better in performance
   compared to the AVX2 generation.

This update improves bluestore compression and potential to improve
dedup performance.

Testing this patch with objectstore fio yeided a max of 37% increase in
compression performance.

Fio Params:
rw=randwrite,buffer_compress_percentage=50,nr_files=64,direct=1,buffered=0,size=4G,bs=64k

Test			Avg BlueStore Compression Time
			v2.16(us)  v2.18(us)
iodepth=1,jobs=1	241.658    175.476	27.39%
iodepth=2,jobs=2	184.174    145.861	20.80%
iodepth=4,jobs=4	143.617    104.392	27.31%
iodepth=8,jobs=8	146.984    116.505	20.74%
iodepth=16,jobs=8	180.167    112.769	37.41%

Signed-off-by: Ganesh Mahalingam <ganesh.mahalingam@intel.com>
Signed-off-by: Tushar Gohad <tushar.gohad@intel.com>
@ganeshmaharaj

This comment has been minimized.

Show comment
Hide comment
@ganeshmaharaj

ganeshmaharaj Jul 6, 2017

Contributor

@liewegas @tchaikov Updated the patch to include the perf numbers in the commit message against a non-permanent google doc. The data provided is the bluestore compression time as provided by bluestore when tested with objectstore fio plugin.

Contributor

ganeshmaharaj commented Jul 6, 2017

@liewegas @tchaikov Updated the patch to include the perf numbers in the commit message against a non-permanent google doc. The data provided is the bluestore compression time as provided by bluestore when tested with objectstore fio plugin.

@tchaikov tchaikov merged commit 5485e6b into ceph:master Jul 7, 2017

3 of 4 checks passed

Unmodified Submodules Approval needed: modified submodules found
Details
Signed-off-by all commits in this PR are signed
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details
@ganeshmaharaj

This comment has been minimized.

Show comment
Hide comment
@ganeshmaharaj

ganeshmaharaj Jul 7, 2017

Contributor

Not sure if this is a thing of concern, but thought i will bring it to your notice. Any existing old ceph repos that folks use will run into the below error if they just do git submodule update --recursive after this change has been merged.

ganeshma|Illidan|/data/odisk/ceph/ceph >>> git submodule update --recursive 
error: Server does not allow request for unadvertised object e1a337433a340bc0974ed0f04301bdaca374af6
Fetched in submodule path 'src/isa-l', but it did not contain e1a337433a340bc0974ed0f04301bdaca374af6. Direct fetching of that commit failed.

This I believe is due to the fact that no branch is tracking the SHA-1 of the tag v2.18.0. The way i could fix it was with the below commands from my ceph repo top_dir

cd src/isa-l/
git fetch origin --tags
cd ../../
git submodule update --recursive

If it is not an issue, updating the master branch in https://github.com/ceph/isa-l to the commit tagged v2.18.0 should eliminate this error msg.

Contributor

ganeshmaharaj commented Jul 7, 2017

Not sure if this is a thing of concern, but thought i will bring it to your notice. Any existing old ceph repos that folks use will run into the below error if they just do git submodule update --recursive after this change has been merged.

ganeshma|Illidan|/data/odisk/ceph/ceph >>> git submodule update --recursive 
error: Server does not allow request for unadvertised object e1a337433a340bc0974ed0f04301bdaca374af6
Fetched in submodule path 'src/isa-l', but it did not contain e1a337433a340bc0974ed0f04301bdaca374af6. Direct fetching of that commit failed.

This I believe is due to the fact that no branch is tracking the SHA-1 of the tag v2.18.0. The way i could fix it was with the below commands from my ceph repo top_dir

cd src/isa-l/
git fetch origin --tags
cd ../../
git submodule update --recursive

If it is not an issue, updating the master branch in https://github.com/ceph/isa-l to the commit tagged v2.18.0 should eliminate this error msg.

@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas Jul 7, 2017

Member
Member

liewegas commented Jul 7, 2017

@ganeshmaharaj

This comment has been minimized.

Show comment
Hide comment
@ganeshmaharaj

ganeshmaharaj Jul 7, 2017

Contributor

Thanks a lot @liewegas

Contributor

ganeshmaharaj commented Jul 7, 2017

Thanks a lot @liewegas

@ganeshmaharaj ganeshmaharaj deleted the ganeshmaharaj:isal-v2.18 branch Jul 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment