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

use lz4 instead of gzip for StringEncoding compress/decompress #1196

Merged
merged 11 commits into from
Oct 26, 2020

Conversation

zhoney
Copy link
Contributor

@zhoney zhoney commented Sep 28, 2020

No description provided.

Change-Id: Ie4458ba481ac3625139458cc92e9dbc0631898da
Change-Id: I39cbfe2f643b46ca736e5076ef73ec6203b0be3a
@codecov
Copy link

codecov bot commented Sep 28, 2020

Codecov Report

Merging #1196 into master will increase coverage by 0.01%.
The diff coverage is 71.42%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1196      +/-   ##
============================================
+ Coverage     65.75%   65.76%   +0.01%     
+ Complexity     5754     5753       -1     
============================================
  Files           361      360       -1     
  Lines         29548    29593      +45     
  Branches       4172     4180       +8     
============================================
+ Hits          19428    19463      +35     
- Misses         8147     8152       +5     
- Partials       1973     1978       +5     
Impacted Files Coverage Δ Complexity Δ
...aidu/hugegraph/backend/serializer/BytesBuffer.java 87.75% <60.00%> (-0.18%) 175.00 <3.00> (ø)
...rc/main/java/com/baidu/hugegraph/util/LZ4Util.java 70.00% <66.66%> (+70.00%) 3.00 <3.00> (+3.00)
.../java/com/baidu/hugegraph/util/StringEncoding.java 85.07% <77.77%> (+0.65%) 29.00 <5.00> (+2.00)
...c/main/java/com/baidu/hugegraph/task/HugeTask.java 69.84% <100.00%> (ø) 82.00 <0.00> (ø)
...idu/hugegraph/backend/store/raft/StoreCommand.java 56.52% <0.00%> (-31.72%) 6.00% <0.00%> (ø%)
.../hugegraph/backend/store/raft/StoreSerializer.java 58.73% <0.00%> (-1.93%) 6.00% <0.00%> (ø%)
...ph/backend/store/AbstractBackendStoreProvider.java 92.59% <0.00%> (-1.16%) 23.00% <0.00%> (ø%)
...m/baidu/hugegraph/backend/store/raft/RaftNode.java 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
...hugegraph/backend/store/raft/RaftBackendStore.java 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
...ugegraph/backend/store/raft/RaftSharedContext.java 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db8bb17...3751061. Read the comment docs.

Copy link
Contributor

@javeme javeme left a comment

Choose a reason for hiding this comment

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

explain the effect in commit message

buf.write(bytes, 0, len);
}
return decode(buf.bytes());
return decode(LZ4Util.decompress(buf.bytes(), BLOCK_SIZE).array());
Copy link
Contributor

Choose a reason for hiding this comment

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

to avoid copy bytes, add method decode(bytes, offset, length), and let BytesBuffer implements OutputStream that BytesBuffer instance could be pass into LZ4BlockOutputStream.

Copy link
Contributor

Choose a reason for hiding this comment

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

address it:
use BytesBuffer instead of ByteArrayOutputStream in compress and decompress method, and init BytesBuffer estimate size

byte[] bytes = new byte[value.length];
int len;
while ((len = in.read(bytes)) > 0) {
while ((len = bis.read(bytes)) > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to write a buf, just LZ4Util.decompress(value)

out.write(bytes);
out.finish();
bytes = LZ4Util.compress(bytes, BLOCK_SIZE).array();
bos.write(bytes);
Copy link
Contributor

Choose a reason for hiding this comment

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

just return LZ4Util.compress(bytes, BLOCK_SIZE).bytes()

Change-Id: I408bc15c3be531239c92b3da1d75af192a532347
backend=${backend}
serializer=${serializer}
backend=rocksdb
serializer=binary
Copy link
Contributor

Choose a reason for hiding this comment

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

keep origin

buf.write(bytes, 0, len);
}
return decode(buf.bytes());
return decode(LZ4Util.decompress(buf.bytes(), BLOCK_SIZE).array());
Copy link
Contributor

Choose a reason for hiding this comment

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

address it:
use BytesBuffer instead of ByteArrayOutputStream in compress and decompress method, and init BytesBuffer estimate size

Change-Id: I812c0ba5a2abda0d11cb2e7af5ebc00da0c26ef6
@@ -135,11 +144,13 @@ public static String encodeBase64(byte[] bytes) {
}

public static byte[] compress(String value) {
return LZ4Util.compress(encode(value), BLOCK_SIZE).array();
BytesBuffer bf = LZ4Util.compress(encode(value), BLOCK_SIZE);
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to buf

}

public static String decompress(byte[] value) {
return decode(LZ4Util.decompress(value, BLOCK_SIZE).array());
BytesBuffer bf = LZ4Util.decompress(value, BLOCK_SIZE);
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

ByteArrayInputStream bais = new ByteArrayInputStream(bytes);
ByteArrayOutputStream baos = new ByteArrayOutputStream(blockSize);
BytesBuffer bf = new BytesBuffer(bytes.length * 8);
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to buf

@@ -37,35 +36,35 @@
public static BytesBuffer compress(byte[] bytes, int blockSize) {
LZ4Factory factory = LZ4Factory.fastestInstance();
LZ4Compressor compressor = factory.fastCompressor();
ByteArrayOutputStream baos = new ByteArrayOutputStream();
BytesBuffer bf = new BytesBuffer(bytes.length / 8);
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to buf

require(BYTE_LEN * val.length);
this.buffer.put(val);
return this;
}

public BytesBuffer write(byte[] val, int offset, int length) {
public BytesBuffer writeByteArray(byte[] val, int offset, int length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer keep these methods returning void

Change-Id: I3bc4db83bdf3d6eed14567a771f25c744d486d44
}

public BytesBuffer write(byte[] val, int offset, int length) {
public void write(byte[] val, int offset, int length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

add "@OverRide" for 3 write() methods

Change-Id: Ia3b3b4cff5cb4967815deeb27d0bcd5093d3c75e
@@ -36,7 +36,7 @@
public static BytesBuffer compress(byte[] bytes, int blockSize) {
LZ4Factory factory = LZ4Factory.fastestInstance();
LZ4Compressor compressor = factory.fastCompressor();
BytesBuffer buf = new BytesBuffer(bytes.length / 8);
BytesBuffer buf = new BytesBuffer(bytes.length);
Copy link
Contributor

Choose a reason for hiding this comment

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

add a const var buf_ratio, let init_size=bytes.length/buf_ratio, also update decompress to bytes.length*buf_ratio

Change-Id: Ie15e6bf9c6eed48a448c4ed84d35e4f27197b67b
Change-Id: Ib68648def8946433666211b25782775151fb4c52
Change-Id: I0919c57e990f50a0df18a4d942d2de67947f928e
Copy link
Contributor

@javeme javeme left a comment

Choose a reason for hiding this comment

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

ci failed

return compress(value, 0.0F);
}

public static byte[] compress(String value, float ratio) {
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer to not add this method

return decompress(value, 0.0F);
}

public static String decompress(byte[] value, float ratio) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor

Choose a reason for hiding this comment

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

rename to bufferRatio

@zhoney
Copy link
Contributor Author

zhoney commented Oct 26, 2020

cassandra

[ERROR] Tests run: 683, Failures: 1, Errors: 0, Skipped: 21, Time elapsed: 482.552 s <<< FAILURE! - in com.baidu.hugegraph.core.CoreTestSuite
[ERROR] testAddEdgeWithTtlAndTtlStartTime(com.baidu.hugegraph.core.EdgeCoreTest)  Time elapsed: 1.796 s  <<< FAILURE!
java.lang.AssertionError
	at com.baidu.hugegraph.core.EdgeCoreTest.testAddEdgeWithTtlAndTtlStartTime(EdgeCoreTest.java:754)

hbase

[ERROR] Tests run: 38, Failures: 3, Errors: 5, Skipped: 0, Time elapsed: 13.07 s <<< FAILURE! - in com.baidu.hugegraph.api.ApiTestSuite
[ERROR] testList(com.baidu.hugegraph.api.TaskApiTest)  Time elapsed: 0.103 s  <<< FAILURE!
java.lang.AssertionError: Response with status 400 and content {"exception":"class com.baidu.hugegraph.exception.ExistedException","message":"The index label 'personByCity' has existed","cause":""} expected:<202> but was:<400>
	at com.baidu.hugegraph.api.TaskApiTest.prepareSchema(TaskApiTest.java:42)
[ERROR] testList(com.baidu.hugegraph.api.TaskApiTest)  Time elapsed: 0.103 s  <<< ERROR!
com.baidu.hugegraph.HugeException: Failed to list indexlabels
[ERROR] testCancel(com.baidu.hugegraph.api.TaskApiTest)  Time elapsed: 0.058 s  <<< FAILURE!
java.lang.AssertionError: Response with status 400 and content {"exception":"class com.baidu.hugegraph.exception.ExistedException","message":"The index label 'personByCity' has existed","cause":""} expected:<202> but was:<400>
	at com.baidu.hugegraph.api.TaskApiTest.prepareSchema(TaskApiTest.java:42)
[ERROR] testCancel(com.baidu.hugegraph.api.TaskApiTest)  Time elapsed: 0.058 s  <<< ERROR!
com.baidu.hugegraph.HugeException: Failed to list indexlabels
[ERROR] testDelete(com.baidu.hugegraph.api.TaskApiTest)  Time elapsed: 0.033 s  <<< FAILURE!
java.lang.AssertionError: Response with status 400 and content {"exception":"class com.baidu.hugegraph.exception.ExistedException","message":"The index label 'personByCity' has existed","cause":""} expected:<202> but was:<400>
	at com.baidu.hugegraph.api.TaskApiTest.prepareSchema(TaskApiTest.java:42)
[ERROR] testDelete(com.baidu.hugegraph.api.TaskApiTest)  Time elapsed: 0.034 s  <<< ERROR!
com.baidu.hugegraph.HugeException: Failed to list indexlabels
[ERROR] com.baidu.hugegraph.api.GremlinApiTest  Time elapsed: 0.122 s  <<< ERROR!
com.baidu.hugegraph.HugeException: Failed to list indexlabels
[ERROR] com.baidu.hugegraph.api.MetricsApiTest  Time elapsed: 0.191 s  <<< ERROR!
com.baidu.hugegraph.HugeException: Failed to list indexlabels
[INFO] 
[INFO] Results:
[INFO] 
[ERROR] Errors: 
[ERROR]   GremlinApiTest>BaseApiTest.init:71->BaseApiTest.clearData:459->BaseApiTest.clearSchema:417->BaseApiTest.lambda$clearSchema$5:396 » Huge
[ERROR]   MetricsApiTest>BaseApiTest.init:71->BaseApiTest.clearData:459->BaseApiTest.clearSchema:417->BaseApiTest.lambda$clearSchema$5:396 » Huge
[ERROR] com.baidu.hugegraph.api.TaskApiTest.testCancel(com.baidu.hugegraph.api.TaskApiTest)
[ERROR]   Run 1: TaskApiTest.prepareSchema:42->BaseApiTest.initIndexLabel:279->BaseApiTest.assertResponseStatus:476 Response with status 400 and content {"exception":"class com.baidu.hugegraph.exception.ExistedException","message":"The index label 'personByCity' has existed","cause":""} expected:<202> but was:<400>
[ERROR]   Run 2: TaskApiTest>BaseApiTest.teardown:81->BaseApiTest.clearData:459->BaseApiTest.clearSchema:417->BaseApiTest.lambda$clearSchema$5:396 » Huge
[INFO] 
[ERROR] com.baidu.hugegraph.api.TaskApiTest.testDelete(com.baidu.hugegraph.api.TaskApiTest)
[ERROR]   Run 1: TaskApiTest.prepareSchema:42->BaseApiTest.initIndexLabel:279->BaseApiTest.assertResponseStatus:476 Response with status 400 and content {"exception":"class com.baidu.hugegraph.exception.ExistedException","message":"The index label 'personByCity' has existed","cause":""} expected:<202> but was:<400>
[ERROR]   Run 2: TaskApiTest>BaseApiTest.teardown:81->BaseApiTest.clearData:459->BaseApiTest.clearSchema:417->BaseApiTest.lambda$clearSchema$5:396 » Huge
[INFO] 
[ERROR] com.baidu.hugegraph.api.TaskApiTest.testList(com.baidu.hugegraph.api.TaskApiTest)
[ERROR]   Run 1: TaskApiTest.prepareSchema:42->BaseApiTest.initIndexLabel:279->BaseApiTest.assertResponseStatus:476 Response with status 400 and content {"exception":"class com.baidu.hugegraph.exception.ExistedException","message":"The index label 'personByCity' has existed","cause":""} expected:<202> but was:<400>
[ERROR]   Run 2: TaskApiTest>BaseApiTest.teardown:81->BaseApiTest.clearData:459->BaseApiTest.clearSchema:417->BaseApiTest.lambda$clearSchema$5:396 » Huge

Change-Id: If332999b94a2ef918b2b9d125a99de6ae15b59fe
} catch (IOException e) {
throw new BackendException("Failed to compress: %s", e, value);
}
return compress(value, 0.0F);
Copy link
Contributor

Choose a reason for hiding this comment

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

define const

return decompress(value, 0.0F);
}

public static String decompress(byte[] value, float ratio) {
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to bufferRatio

Change-Id: I1970aab20cea373a6e5e1c11dea364aa38b7d3d4
@zhoney zhoney merged commit 0e4eaa0 into master Oct 26, 2020
@zhoney zhoney deleted the task-result-lz4 branch October 26, 2020 10:58
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.

None yet

3 participants