lostluck commented on a change in pull request #11257: [BEAM-9642] Create runtime invokers for SDF methods. URL: https://github.com/apache/beam/pull/11257#discussion_r401779838
########## File path: sdks/go/pkg/beam/core/runtime/exec/sdf_invokers.go ########## @@ -0,0 +1,302 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package exec + +import ( + "github.com/apache/beam/sdks/go/pkg/beam/core/funcx" + "github.com/apache/beam/sdks/go/pkg/beam/core/sdf" + "github.com/apache/beam/sdks/go/pkg/beam/core/util/reflectx" + "github.com/apache/beam/sdks/go/pkg/beam/internal/errors" + "reflect" +) + +// This file contains invokers for SDF methods. These invokers are based off +// exec.invoker which is used for regular DoFns. Since exec.invoker is +// specialized for DoFns it cannot be used for SDF methods. Instead, these +// invokers pare down the functionality to only what is essential for +// executing SDF methods, including per-element optimizations. +// +// Each SDF method invoker in this file is specific to a certain method, but +// they are all used the same way. Create an invoker with new[Method]Invoker +// in the Up method of an exec.Unit, and then invoke it with Invoke. Finally, +// call Reset on it when the bundle ends in FinishBundle. +// +// These invokers are not thread-safe. + +// cirInvoker is an invoker for CreateInitialRestriction. +type cirInvoker struct { + fn *funcx.Fn + args []interface{} // Cache to avoid allocating new slices per-element. + call func(elms *FullValue) (rest interface{}) +} + +func newCreateInitialRestrictionInvoker(fn *funcx.Fn) (*cirInvoker, error) { + n := &cirInvoker{ + fn: fn, + args: make([]interface{}, len(fn.Param)), + } + if err := n.initCallFn(); err != nil { + return nil, errors.WithContext(err, "sdf CreateInitialRestriction invoker") + } + return n, nil +} + +func (n *cirInvoker) initCallFn() error { + // Expects a signature of the form: + // (key?, value) restriction + // TODO(BEAM-9643): Link to full documentation. + switch fnT := n.fn.Fn.(type) { + case reflectx.Func1x1: + n.call = func(elms *FullValue) interface{} { + return fnT.Call1x1(elms.Elm) + } + case reflectx.Func2x1: + n.call = func(elms *FullValue) interface{} { + return fnT.Call2x1(elms.Elm, elms.Elm2) + } + default: + switch len(n.fn.Param) { + case 1: + n.call = func(elms *FullValue) interface{} { + n.args[0] = elms.Elm + return n.fn.Fn.Call(n.args)[0] + } + case 2: + n.call = func(elms *FullValue) interface{} { + n.args[0] = elms.Elm + n.args[1] = elms.Elm2 + return n.fn.Fn.Call(n.args)[0] + } + default: + return errors.Errorf("CreateInitialRestriction fn %v has unexpected number of parameters: %v", + n.fn.Fn.Name(), len(n.fn.Param)) + } + } + + return nil +} + +// Invoke calls CreateInitialRestriction with the given FullValue as the element +// and returns the resulting restriction. +func (n *cirInvoker) Invoke(elms *FullValue) (rest interface{}) { + return n.call(elms) +} + +// Reset zeroes argument entries in the cached slice to allow values to be +// garbage collected after the bundle ends. +func (n *cirInvoker) Reset() { + for i := range n.args { + n.args[i] = nil + } +} + +// srInvoker is an invoker for SplitRestriction. +type srInvoker struct { + fn *funcx.Fn + args []interface{} // Cache to avoid allocating new slices per-element. Review comment: No action required, just commentary. Optimization note: We know this will never be more than 3 (or similar for the other methods), so it could be used as an actual Array instead of a slice [3]interface{}, and we could similarly simply unroll the loops or, even full clear it by assigning a new instance in Reset for maximum efficiency. (eg. n.args = [3]interface{}{}) The tricky bit is we still arguably should cache the slice part itself since that could repeatedly get heap allocated on each call. This doesn't work for the general invoker since the number of emit functions could be arbitrary, and we don't gain as much from it. ---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
