>From Ali Alsuliman <[email protected]>: Attention is currently required from: Ian Maxon. Ali Alsuliman has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17864 )
Change subject: [ASTERIX-3283][RT] Profile micro-ops and subplans ...................................................................... Patch Set 7: (5 comments) File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/common/ClosedRecordConstructorEvalFactory.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17864/comment/d10ecc0c_4f567add PS5, Line 43: @Override : public String toString() { : return "ClosedRecordConstructor"; : } > this is just to improve the appearance of the output of the profile, as it > uses tostring for every f […] Can you move the method below the constructor? File hyracks-fullstack/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/base/ProfiledPushRuntime.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17864/comment/c5ef9722_b28cde89 PS7, Line 47: public ProfiledPushRuntime(IPushRuntime push, IOperatorStats stats) { Remove unused constructor. File hyracks-fullstack/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/base/AbstractPushRuntimeFactory.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17864/comment/6bf9ca5d_89d2fc3c PS7, Line 27: private static final long serialVersionUID = 2L; Do we need this change? File hyracks-fullstack/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/meta/AlgebricksMetaOperatorDescriptor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17864/comment/1832cbd4_3d468790 PS7, Line 280: pipeline.getRuntimeFactories()[0] Shouldn't this be the last runtime factory based on how getStats() is used? getStats() is called so that the stats is set by the destination wrapper "next writer" as upstream (parent). File hyracks-fullstack/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/meta/PipelineAssembler.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17864/comment/f78ef9f7_d2426cb1 PS7, Line 75: IOperatorStats prevStats = parentStats; The parentStats passed by the callers are not correct, I believe. In the for loop below, we walk backwards starting from the last push runtime. You should do it similar to how ProfiledOperatorNodePushable.setOutputFrameWriter() does it. i.e. in the ProfiledPushRuntime.setOutputFrameWriter(), set the upstream stats. -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17864 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: trinity Gerrit-Change-Id: Ib266878bf05782506045abfadaa83b41f0f9598b Gerrit-Change-Number: 17864 Gerrit-PatchSet: 7 Gerrit-Owner: Ian Maxon <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Attention: Ian Maxon <[email protected]> Gerrit-Comment-Date: Tue, 14 Nov 2023 05:28:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Ian Maxon <[email protected]> Comment-In-Reply-To: Ali Alsuliman <[email protected]> Gerrit-MessageType: comment
