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: (1 comment) 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@452 PS2, Line 452: NativeLz4Internal(JNIEnv* env, jbyteArray src, jint src_off, jint src_len, PhilZ, this is code that the library would handle for us. I don't think it's worth pulling in a separate library for these simple few lines. Also, the error handling in this function is better than in the library, imo. The library throws an OOM for the L455 and L458 cases here without calling ReleasePrimitiveArrayCritical() which seems bad. -- 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: Philip Zeyliger <[email protected]> Gerrit-Comment-Date: Thu, 28 Sep 2017 17:25:32 +0000 Gerrit-HasComments: Yes
