impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5990: Part 1: JNI-based LZ4 de/compression
Date Sat, 30 Sep 2017 04:23:52 GMT
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8150 )

Change subject: IMPALA-5990: Part 1: JNI-based LZ4 de/compression
......................................................................


Patch Set 2:

(2 comments)

Just realized I forgot these comments, sorry.

http://gerrit.cloudera.org:8080/#/c/8150/2/be/src/service/fe-support.cc
File be/src/service/fe-support.cc:

http://gerrit.cloudera.org:8080/#/c/8150/2/be/src/service/fe-support.cc@454
PS2, Line 454: 0
> I'm not fully familiar with how it works internally, but per my reading of 
Why would it need to free the copied bytes? The copied bytes are on the Java heap and will
be GC'ed as usual (just like the original array once the GC is unlocked again)


http://gerrit.cloudera.org:8080/#/c/8150/2/fe/src/main/java/org/apache/impala/service/FeSupport.java
File fe/src/main/java/org/apache/impala/service/FeSupport.java:

http://gerrit.cloudera.org:8080/#/c/8150/2/fe/src/main/java/org/apache/impala/service/FeSupport.java@306
PS2, Line 306: ByteBuffer.wrap(dst).putInt(0, payload.length);
             :     ByteBuffer.wrap(dst).putInt(4, compressedSize);
> Ok, sorry for not being clear. My point is, why have two versions, Lz4Compr
We may not need the ByteBuffer version. Removed.



-- 
To view, visit http://gerrit.cloudera.org:8080/8150
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I237802944875b07080db0159ff8ec548150fd95e
Gerrit-Change-Number: 8150
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bharathv@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <philip@cloudera.com>
Gerrit-Comment-Date: Sat, 30 Sep 2017 04:23:52 +0000
Gerrit-HasComments: Yes

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message