[GitHub] [beam] youngoli commented on a change in pull request #13070: [BEAM-11020] Adding multi-window splitting to Go SDF.

[GitBox <gi...@apache.org>]

youngoli commented on a change in pull request #13070:
URL: https://github.com/apache/beam/pull/13070#discussion_r503526693



##########
File path: sdks/go/pkg/beam/core/runtime/exec/sdf.go
##########
@@ -338,12 +348,17 @@ func (n *ProcessSizedElementsAndRestrictions) ProcessElement(_ context.Context,
 		// If we need to process the element in multiple windows, each one needs
 		// its own RTracker and progress must be tracked among all windows by
 		// currW updated between processing.
-		for _, w := range elm.Windows {
+		n.numW = len(elm.Windows)
+
+		//for _, w := range elm.Windows {

Review comment:
       Removed the commented out line. It's actually not identical because a dynamic split can modify n.numW if some of the windows are split with the residual, which is why I loop using numW instead. I could theoretically loop based on `len(elm.Windows)` instead, avoiding the `range` keyword, and directly trim the windows slice in the current element, but I don't know if there's any advantage to doing it that way and this seems simpler.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org