Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/15236 )
Change subject: IMPALA-9381: lazy conversion of runtime profile ...................................................................... Patch Set 3: Code-Review+1 (4 comments) http://gerrit.cloudera.org:8080/#/c/15236/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15236/3//COMMIT_MSG@7 PS3, Line 7: lazy I would replace "lazy" with "On demand" - to me lazy suggests that it is constructed only when needed, but it is stored from that point. http://gerrit.cloudera.org:8080/#/c/15236/3/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/15236/3/be/src/service/impala-server.cc@826 PS3, Line 826: record If you are already optimizing this path, then it would be nice to avoid this copy, e.g. by storing unique_ptr<QueryStateRecord>s in query_log_. http://gerrit.cloudera.org:8080/#/c/15236/3/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: http://gerrit.cloudera.org:8080/#/c/15236/3/be/src/util/runtime-profile.cc@1114 PS3, Line 1114: scoped_ptr<Codec> decompressor; : MemTracker mem_tracker; : MemPool mem_pool(&mem_tracker); : const auto close_mem_tracker = MakeScopeExitTrigger([&mem_pool, &mem_tracker]() { : mem_pool.FreeAll(); : mem_tracker.Close(); : }); nice to have: it could be useful during prototyping to move this logic to a new class, e.g. SimpleDecompressor (also applies to the Compressor logic). http://gerrit.cloudera.org:8080/#/c/15236/3/be/src/util/runtime-profile.cc@1122 PS3, Line 1122: DEFAULT Using DEFAULT seems weird to me - I guess that the are tools that parse these profile expect a specific compression. -- To view, visit http://gerrit.cloudera.org:8080/15236 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic2f5133cc146adc3b044cf4b64aae0a9688449fa Gerrit-Change-Number: 15236 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Thu, 20 Feb 2020 18:11:08 +0000 Gerrit-HasComments: Yes
