>From Ali Alsuliman <[email protected]>: Attention is currently required from: Ali Alsuliman, Ian Maxon.
Ali Alsuliman has posted comments on this change by Ian Maxon. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20270?usp=email ) Change subject: [ASTERIXDB-3631][RT] Profile nested groupby clauses ...................................................................... Patch Set 8: (5 comments) File hyracks-fullstack/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/aggreg/NestedPlansAccumulatingAggregatorFactory.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20270/comment/eb52b268_246d49cc?usp=email : PS8, Line 186: profiling(ctx) ? new ArrayList<>(subplans.length) : Collections.emptyList(); You can call profiling(ctx) only once and re-use in the 3 places. 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/+/20270/comment/03c5bfdd_881cec06?usp=email : PS8, Line 187: boolean shouldProfile = profile && !(runtimeFactory instanceof EmptyTupleSourceRuntimeFactory) If we are here, then profile must be true, same thing for below. File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/job/profiling/StatsCollector.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20270/comment/04fda654_fb670312?usp=email : PS8, Line 38: private final Map<String, IOperatorStats> operatorStatsMap = new ConcurrentHashMap<>(); Don't we need to use synchronized blocks or synchronized map? My knowledge is that concurrent hash map does not establish happens-before by itself if the two threads (writing thread and reading thread) are not joined somehow. File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/group/preclustered/PreclusteredGroupOperatorNodePushable.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20270/comment/b854b98c_53a19798?usp=email : PS8, Line 41: private final RecordDescriptor outRecordDescriptor; Just checking; we also have AbstractUnaryInputUnaryOutputIntrospectingOperatorNodePushable. See if we can extend it. File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/group/preclustered/PreclusteredGroupWriter.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20270/comment/4a60a167_d1d01712?usp=email : PS8, Line 93: aggregatorFactory.setOperatorStats(stats); Why not the aggregator after it's created? e.g.: this.aggregator.setOperatorStats(stats). We can also explore if IProfiledAggregatorDescriptor can have that method as part of the interface. -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20270?usp=email To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: I516dc90b8da3a7086dc80b67946ac14f6ade0973 Gerrit-Change-Number: 20270 Gerrit-PatchSet: 8 Gerrit-Owner: Ian Maxon <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[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-Attention: Ali Alsuliman <[email protected]> Gerrit-Comment-Date: Tue, 16 Dec 2025 03:16:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No
