Tim Armstrong has posted comments on this change. Change subject: IMPALA-3166: basic perf support and asm dumps for codegened code ......................................................................
Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/2793/2/be/src/codegen/codegen-symbol-emitter.cc File be/src/codegen/codegen-symbol-emitter.cc: Line 58: ok > why is that? does perf look at the last function that covers an address or I'm not actually sure. Anyway, I fixed this so that it removes stale data from the map, so it should be easier to reason about. Line 61: perf_map_file.open(perf_map_path, fstream::out | fstream::app); > Does this do that? when does NotifyObjectEmitted get called (function comme I added the function comment. It gets called when the machine code is generated (when FinalizeModule() is called). I made the changes necessary to create the empty file at startup. Unfortunately perf's treatment of the map files is totalyl undocumented. With the changes it seems like it sometimes fails to pick up symbols. http://gerrit.cloudera.org:8080/#/c/2793/2/be/src/codegen/codegen-symbol-emitter.h File be/src/codegen/codegen-symbol-emitter.h: Line 47: void NotifyFreeingObject(const llvm::object::ObjectFile &obj) override; > can you add a brief comment for these two overrides? Done -- To view, visit http://gerrit.cloudera.org:8080/2793 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If25de61e46f4db005956686cddbd4d71a1424528 Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Skye Wanderman-Milne <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
