Skip to content

feat(Go): Implement ahead of time codegen for fory-go serialization - #2553

Merged
chaokunyang merged 11 commits into
apache:mainfrom
ThisingL:feat-Go-codegen
Sep 4, 2025
Merged

feat(Go): Implement ahead of time codegen for fory-go serialization#2553
chaokunyang merged 11 commits into
apache:mainfrom
ThisingL:feat-Go-codegen

Conversation

@ThisingL

Copy link
Copy Markdown
Contributor

What does this PR do?

  • Generates serialization code for structs using the go/codegen package.
  • Implements Write and Read methods for struct types to support serialization and deserialization.
  • Integrates go:generate directive to automate the generation of the serialization code.

Note: This PR only supports basic struct types and does not yet handle map or slice types.

TODO

  • Extend the code generation to handle slice types (planned for the next PR).
  • Implement support for map types (planned for a future PR).

Related issues

Does this PR introduce any user-facing change?

  • Does this PR introduce any public API change?
  • Does this PR introduce any binary protocol compatibility change?

Benchmark

@ThisingL
ThisingL requested a review from chaokunyang as a code owner August 30, 2025 10:08
Comment thread go/fory/codegen/main.go Outdated
// specific language governing permissions and limitations
// under the License.

package main

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the name of this file needs to be modified

Comment thread go/fory/codegen_tests/structs.go Outdated

package codegen_tests

// 验证用的基本结构体 (只包含基本类型,因为PR1只支持基本类型)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is more appropriate to use English annotations

fmt.Printf(" B 匹配: %t (%s == %s)\n", bMatch, result.B, original.B)
fmt.Printf(" C 匹配: %t (%d == %d)\n", cMatch, result.C, original.C)

if aMatch && bMatch && cMatch {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This testing method is unreasonable. Please refer to other testing documents

Comment thread go/fory/go.mod Outdated
github.com/stretchr/objx v0.1.0 // indirect
github.com/yuin/goldmark v1.4.13 // indirect
golang.org/x/crypto v0.41.0 // indirect
golang.org/x/mod v0.27.0 // indirect

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import method seems not elegant enough

Comment thread go/fory/go.mod Outdated
module github.com/apache/fory/go/fory

go 1.13
go 1.23.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not upgrade the version

Comment thread go/fory/go.mod Outdated
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/spaolacci/murmur3 v1.1.0
github.com/stretchr/testify v1.7.0
github.com/stretchr/testify v1.6.1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that lowering the version makes no sense

Comment thread go/fory/codegen/forygen.go Outdated
Comment thread go/fory/codegen/forygen.go Outdated
// specific language governing permissions and limitations
// under the License.

package main

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think this name is inappropriate

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forygen.go shoudl be named as generator.go, and you should have a main.go under go/fory. And in that file, you call into this file

@chaokunyang

chaokunyang commented Sep 3, 2025

Copy link
Copy Markdown
Collaborator

@ThisingL Please add detailed document about how to use this feature in go/README.md. It should contains following content:

  • explain the rationals about fory go codegen, and why fory use this pattern, and also leave a comment this is not mandatory
  • How to install fory go, we need go install xxx for the generator in GOPATH
  • How to run go generate and when to rerun
  • A simple example about generated code to showcase what fory doo under the hood
  • Whether check in generated code to source version control system like git/svn. Or add go generate to release pipeline of user's system

@chaokunyang

Copy link
Copy Markdown
Collaborator

And we should generate some kind of compilation guard, when users update their struct definition, they must reexecute go generate, otherwise the whole code space should fail to compile. See more in https://eli.thegreenplace.net/2021/a-comprehensive-guide-to-go-generate
image

// specific language governing permissions and limitations
// under the License.

// Code generated by forygen. DO NOT EDIT.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file should not be check in to github, yo ushould run go generate in ci script under ci and .github/workflows

Comment thread go/fory/codegen_tests/structs.go Outdated
// specific language governing permissions and limitations
// under the License.

package codegen_tests

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please move this test into tests, it's unnecessary to have another test package

Comment thread go/fory/codegen/forygen.go Outdated
}

// Generate the strongly-typed Read method according to the doc
func generateReadTyped(buf *bytes.Buffer, s *StructInfo) error {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we make this generate more modular, and separate it into several files? the generator can be the entrance

"github.com/stretchr/testify/require"
)

//go:generate go run ../codegen/forygen.go -pkg . -type "ValidationDemo"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think users of fory can specify like this. It's OK to have something like this to make debug more easily. But we also should have annotation like go:generate fory ....

Comment thread go/README.md Outdated

@chaokunyang chaokunyang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@chaokunyang
chaokunyang merged commit cb92dfc into apache:main Sep 4, 2025
59 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants