[ https://issues.apache.org/jira/browse/BEAM-14347?focusedWorklogId=767706&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-767706 ]
ASF GitHub Bot logged work on BEAM-14347: ----------------------------------------- Author: ASF GitHub Bot Created on: 08/May/22 19:05 Start Date: 08/May/22 19:05 Worklog Time Spent: 10m Work Description: lostluck commented on code in PR #17574: URL: https://github.com/apache/beam/pull/17574#discussion_r867529520 ########## sdks/go/pkg/beam/registration/emitter.go: ########## @@ -0,0 +1,173 @@ +// 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 registration + +import ( + "context" + "reflect" + + "github.com/apache/beam/sdks/v2/go/pkg/beam/core/runtime/exec" + "github.com/apache/beam/sdks/v2/go/pkg/beam/core/typex" +) + +type emit1[T any] struct { + n exec.ElementProcessor + + ctx context.Context + ws []typex.Window + et typex.EventTime + value exec.FullValue Review Comment: Another thought: We could make a simple emitImpl and iterImpl types that cover the Init method, and then embed those types, which would deduplicate things a little bit... https://go101.org/article/type-embedding.html But it might not be worthwhile since invoke, and Value() would still need to be hight level type anyway :/. Hmm. ########## sdks/go/pkg/beam/registration/registration_test.go: ########## @@ -155,6 +157,48 @@ func TestRegister_RegistersTypes(t *testing.T) { } } +func TestEmitter1(t *testing.T) { + Emitter1[int]() + if !exec.IsEmitterRegistered(reflect.TypeOf((*func(int))(nil)).Elem()) { + t.Fatalf("exec.IsEmitterRegistered(reflect.TypeOf((*func(int))(nil)).Elem()) = false, want true") + } +} + +func TestEmitter2(t *testing.T) { + Emitter2[int, string]() + if !exec.IsEmitterRegistered(reflect.TypeOf((*func(int, string))(nil)).Elem()) { + t.Fatalf("exec.IsEmitterRegistered(reflect.TypeOf((*func(int, string))(nil)).Elem()) = false, want true") + } +} + +func TestEmitter2_WithTimestamp(t *testing.T) { + Emitter2[typex.EventTime, string]() + if !exec.IsEmitterRegistered(reflect.TypeOf((*func(typex.EventTime, string))(nil)).Elem()) { + t.Fatalf("exec.IsEmitterRegistered(reflect.TypeOf((*func(typex.EventTime, string))(nil)).Elem()) = false, want true") + } +} + +func TestEmitter3(t *testing.T) { + Emitter3[typex.EventTime, int, string]() Review Comment: I just noted coverage is pretty low for the emitters and iterators (12% of the diff), but since the `exec.make*` calls are unexported, that makes it slightly more difficult. I think even instantiating them manually only loses coverage of the factory closures anyway so we're probably fine if we were to do it that way. This way we'll be testing *everything* that is instantiated by users too. Issue Time Tracking ------------------- Worklog Id: (was: 767706) Time Spent: 9h 40m (was: 9.5h) > [Go SDK] Allow users to optimize DoFns with a single generic registration > function > ---------------------------------------------------------------------------------- > > Key: BEAM-14347 > URL: https://issues.apache.org/jira/browse/BEAM-14347 > Project: Beam > Issue Type: New Feature > Components: sdk-go > Reporter: Danny McCormick > Assignee: Danny McCormick > Priority: P2 > Time Spent: 9h 40m > Remaining Estimate: 0h > > Right now, to optimize DoFn execution, users have to use the code generator. > This updates to allow them to use generics instead. -- This message was sent by Atlassian Jira (v8.20.7#820007)