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 <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Comment-Date: Sat, 30 Sep 2017 04:23:52 +0000 Gerrit-HasComments: Yes
