scwhittle commented on code in PR #31317:
URL: https://github.com/apache/beam/pull/31317#discussion_r1622174364
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/StreamingModeExecutionContext.java:
##########
@@ -83,19 +87,30 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-/** {@link DataflowExecutionContext} for use in streaming mode. */
+/**
+ * {@link DataflowExecutionContext} for use in streaming mode. Contains cached
readers and Beam
+ * state pertaining to a processing its owning computation. Can be reused
across processing
+ * different WorkItems for the same computation.
+ */
@SuppressWarnings({
"nullness" // TODO(https://github.com/apache/beam/issues/20497)
Review Comment:
I'm wondering if the separate state via biulder and then accessor instead of
just local variables will lead to performance regression since this is on the
hot path.
Since we don't actually clear the workScopedState between work items it
seems that the null check will only help before start is ever called and then
the additional machinery will be a no-op but more expensive.
Can you revert back to existing local variables for now? Perhaps we can
improve nullability/correctness verification while merging this and the
ComputationWorkExecutor and do a perf comparison. I don't want to make this
big change riskier.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
