Bharath Vissapragada has posted comments on this change. Change subject: [PREVIEW] Use native allocation while building catalog updates ......................................................................
Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/7955/3/fe/src/main/java/org/apache/impala/service/JniCatalog.java File fe/src/main/java/org/apache/impala/service/JniCatalog.java: PS3, Line 142: This method serializes : * the output of CatalogService#getCatallogObjects() into native memory allocated : * using a custom TNativeSerializer and returns the corresponding TNativeByteBuffer : * serialized into bytes. > Alternatively: "Uses a custom serializer (TNativeSerializer) to serialize t Sounds better. http://gerrit.cloudera.org:8080/#/c/7955/3/fe/src/main/java/org/apache/impala/thrift/NativeByteArrayOutputStream.java File fe/src/main/java/org/apache/impala/thrift/NativeByteArrayOutputStream.java: PS3, Line 36: 512MB > update value Removed them, might become stale again if we update. Line 54: // Limit on the size to which the underlying buffer can grow > Comment why that limit exists Done PS3, Line 103: off > offset Done PS3, Line 115: if (bufferLen_ >= BUFFER_DOUBLING_RESIZE_LIMIT) { : while (newBufferSize < bytesWritten_ + len) { : newBufferSize += BUFFER_RESIZE_INCREMENTS; : } : } else { : while (newBufferSize < bytesWritten_ + len) { : newBufferSize <<= 1; : } : } > I think if you put the if/else block inside the while block it may be a bit yea, thats better. PS3, Line 135: public synchronized void reset() { : } > single line (here and for some of the other functions as well). Done -- To view, visit http://gerrit.cloudera.org:8080/7955 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I383684effa9524734ce3c6c0fb7ed37de0e15782 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Mostafa Mokhtar <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
