This is an automated email from the ASF dual-hosted git repository.
chaokunyang pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/fory.git
The following commit(s) were added to refs/heads/main by this push:
new 391727168 refactor(go): refine collection header bitmap in codegen
(#2676)
391727168 is described below
commit 391727168436d77ca8c8ae6870682c6f59a616a1
Author: thisingl <[email protected]>
AuthorDate: Sat Sep 27 18:53:18 2025 +0800
refactor(go): refine collection header bitmap in codegen (#2676)
<!--
**Thanks for contributing to Apache Fory™.**
**If this is your first time opening a PR on fory, you can refer to
[CONTRIBUTING.md](https://github.com/apache/fory/blob/main/CONTRIBUTING.md).**
Contribution Checklist
- The **Apache Fory™** community has requirements on the naming of pr
titles. You can also find instructions in
[CONTRIBUTING.md](https://github.com/apache/fory/blob/main/CONTRIBUTING.md).
- Apache Fory™ has a strong focus on performance. If the PR you submit
will have an impact on performance, please benchmark it first and
provide the benchmark result here.
-->
## Why?
Fix compatibility issues between codegen and reflection paths after the
collection header bitmap refactoring.
<!-- Describe the purpose of this PR. -->
## What does this PR do?
- Update codegen to use the new positive logic bits
(CollectionIsDeclElementType instead of CollectionNotDeclElementType)
- Fix string fields to use NotNullValueFlag instead of RefValueFlag
(aligning with removed string reference tracking)
- Fix slice serialization in codegen to match reflection's collection
flag behavior
- Handle different collection flag combinations correctly in decoder
<!-- Describe the details of this PR. -->
## Related issues
- #2227
<!--
Is there any related issue? If this PR closes them you say say
fix/closes:
- #xxxx0
- #xxxx1
- Fixes #xxxx2
-->
## Does this PR introduce any user-facing change?
<!--
If any user-facing interface changes, please [open an
issue](https://github.com/apache/fory/issues/new/choose) describing the
need to do so and update the document if necessary.
Delete section if not applicable.
-->
- [ ] Does this PR introduce any public API change?
- [ ] Does this PR introduce any binary protocol compatibility change?
## Benchmark
<!--
When the PR has an impact on performance (if you don't know whether the
PR will have an impact on performance, you can submit the PR first, and
if it will have impact on performance, the code reviewer will explain
it), be sure to attach a benchmark data here.
Delete section if not applicable.
-->
---
ci/run_ci.sh | 10 ++--
go/fory/codegen/decoder.go | 93 +++++++++++++++++++++++++----------
go/fory/codegen/encoder.go | 32 +++++-------
go/fory/tests/generator_xlang_test.go | 4 +-
4 files changed, 84 insertions(+), 55 deletions(-)
diff --git a/ci/run_ci.sh b/ci/run_ci.sh
index e18e02728..bbc09feeb 100755
--- a/ci/run_ci.sh
+++ b/ci/run_ci.sh
@@ -356,11 +356,11 @@ case $1 in
;;
go)
echo "Executing fory go tests for go"
- # cd "$ROOT/go/fory"
- # go install ./cmd/fory
- # cd "$ROOT/go/fory/tests"
- # go generate
- # go test -v
+ cd "$ROOT/go/fory"
+ go install ./cmd/fory
+ cd "$ROOT/go/fory/tests"
+ go generate
+ go test -v
cd "$ROOT/go/fory"
go test -v
echo "Executing fory go tests succeeds"
diff --git a/go/fory/codegen/decoder.go b/go/fory/codegen/decoder.go
index 4cf048485..aea956e3d 100644
--- a/go/fory/codegen/decoder.go
+++ b/go/fory/codegen/decoder.go
@@ -146,8 +146,8 @@ func generateFieldReadTyped(buf *bytes.Buffer, field
*FieldInfo) error {
fmt.Fprintf(buf, "\t}\n")
fmt.Fprintf(buf, "\t%s = buf.ReadFloat64()\n",
fieldAccess)
case types.String:
- fmt.Fprintf(buf, "\tif flag := buf.ReadInt8(); flag !=
0 {\n")
- fmt.Fprintf(buf, "\t\treturn fmt.Errorf(\"expected
RefValueFlag for field %s, got %%d\", flag)\n", field.GoName)
+ fmt.Fprintf(buf, "\tif flag := buf.ReadInt8(); flag !=
-1 {\n")
+ fmt.Fprintf(buf, "\t\treturn fmt.Errorf(\"expected
NotNullValueFlag for field %s, got %%d\", flag)\n", field.GoName)
fmt.Fprintf(buf, "\t}\n")
fmt.Fprintf(buf, "\t%s = fory.ReadString(buf)\n",
fieldAccess)
default:
@@ -234,21 +234,21 @@ func generateSliceRead(buf *bytes.Buffer, sliceType
*types.Slice, fieldAccess st
// Read collection flags for non-empty slice
fmt.Fprintf(buf, "\t\t\t// Read collection flags\n")
fmt.Fprintf(buf, "\t\t\tcollectFlag := buf.ReadInt8()\n")
- fmt.Fprintf(buf, "\t\t\t// Check if CollectionNotDeclElementType flag
is set\n")
- fmt.Fprintf(buf, "\t\t\tif (collectFlag & 4) != 0 {\n")
- fmt.Fprintf(buf, "\t\t\t\t// Read element type ID (we expect it but
don't need to validate it for codegen)\n")
+ fmt.Fprintf(buf, "\t\t\t// Check if CollectionIsDeclElementType flag is
NOT set (meaning we need to read type ID)\n")
+ fmt.Fprintf(buf, "\t\t\tif (collectFlag & 4) == 0 {\n")
+ fmt.Fprintf(buf, "\t\t\t\t// Read element type ID (not declared, so we
need to read it)\n")
fmt.Fprintf(buf, "\t\t\t\t_ = buf.ReadVarInt32()\n")
fmt.Fprintf(buf, "\t\t\t}\n")
// Create slice
fmt.Fprintf(buf, "\t\t\t%s = make(%s, sliceLen)\n", fieldAccess,
sliceType.String())
- // Read elements
+ // Read elements - for declared type slices, use direct element reading
without flags
fmt.Fprintf(buf, "\t\t\tfor i := 0; i < sliceLen; i++ {\n")
- // Generate element read code based on type
+ // Generate element read code - for typed slices, read directly via
serializer
elemAccess := fmt.Sprintf("%s[i]", fieldAccess)
- if err := generateSliceElementRead(buf, elemType, elemAccess); err !=
nil {
+ if err := generateSliceElementReadDirect(buf, elemType, elemAccess);
err != nil {
return err
}
@@ -358,33 +358,32 @@ func generateSliceReadInline(buf *bytes.Buffer, sliceType
*types.Slice, fieldAcc
// Read collection header
fmt.Fprintf(buf, "\t\t\tcollectFlag := buf.ReadInt8()\n")
- fmt.Fprintf(buf, "\t\t\t// We expect 12 (no ref tracking) or 13 (with
ref tracking)\n")
- fmt.Fprintf(buf, "\t\t\tif collectFlag != 12 && collectFlag != 13 {\n")
- fmt.Fprintf(buf, "\t\t\t\treturn fmt.Errorf(\"unexpected collection
flag: %%d\", collectFlag)\n")
- fmt.Fprintf(buf, "\t\t\t}\n")
+ fmt.Fprintf(buf, "\t\t\t// Check if CollectionIsDeclElementType is set
(bit 2, value 4)\n")
+ fmt.Fprintf(buf, "\t\t\thasDeclType := (collectFlag & 4) != 0\n")
// Create slice
fmt.Fprintf(buf, "\t\t\t%s = make(%s, sliceLen)\n", fieldAccess,
sliceType.String())
- // Read elements
- fmt.Fprintf(buf, "\t\t\tfor i := 0; i < sliceLen; i++ {\n")
-
- // For each element, read NotNullValueFlag + TypeID + Value
- fmt.Fprintf(buf, "\t\t\t\t// Read element NotNullValueFlag\n")
- fmt.Fprintf(buf, "\t\t\t\tif flag := buf.ReadInt8(); flag != -1 {\n")
- fmt.Fprintf(buf, "\t\t\t\t\treturn fmt.Errorf(\"expected
NotNullValueFlag for element, got %%d\", flag)\n")
- fmt.Fprintf(buf, "\t\t\t\t}\n")
-
- // Read and verify element type ID
- if err := generateElementTypeIDReadInline(buf, elemType); err != nil {
+ // Read elements based on whether CollectionIsDeclElementType is set
+ fmt.Fprintf(buf, "\t\t\tif hasDeclType {\n")
+ fmt.Fprintf(buf, "\t\t\t\t// Elements are written directly without
flags/type IDs\n")
+ fmt.Fprintf(buf, "\t\t\t\tfor i := 0; i < sliceLen; i++ {\n")
+ if err := generateSliceElementReadDirect(buf, elemType,
fmt.Sprintf("%s[i]", fieldAccess)); err != nil {
return err
}
-
- // Read element value
- if err := generateSliceElementReadInline(buf, elemType,
fmt.Sprintf("%s[i]", fieldAccess)); err != nil {
+ fmt.Fprintf(buf, "\t\t\t\t}\n")
+ fmt.Fprintf(buf, "\t\t\t} else {\n")
+ fmt.Fprintf(buf, "\t\t\t\t// Need to read type ID once if
CollectionIsSameType is set\n")
+ fmt.Fprintf(buf, "\t\t\t\tif (collectFlag & 8) != 0 {\n")
+ fmt.Fprintf(buf, "\t\t\t\t\t// Read element type ID once for all
elements\n")
+ fmt.Fprintf(buf, "\t\t\t\t\t_ = buf.ReadVarInt32()\n")
+ fmt.Fprintf(buf, "\t\t\t\t}\n")
+ fmt.Fprintf(buf, "\t\t\t\tfor i := 0; i < sliceLen; i++ {\n")
+ // For same type without declared type, read elements directly
+ if err := generateSliceElementReadDirect(buf, elemType,
fmt.Sprintf("%s[i]", fieldAccess)); err != nil {
return err
}
-
+ fmt.Fprintf(buf, "\t\t\t\t}\n")
fmt.Fprintf(buf, "\t\t\t}\n")
fmt.Fprintf(buf, "\t\t}\n")
fmt.Fprintf(buf, "\t}\n")
@@ -483,6 +482,46 @@ func generateSliceElementReadInline(buf *bytes.Buffer,
elemType types.Type, elem
return fmt.Errorf("unsupported element type for read: %s",
elemType.String())
}
+// generateSliceElementReadDirect generates code to read slice elements
directly via their serializers
+// This is used for typed slices with CollectionIsDeclElementType where no
flags/type IDs are written per element
+func generateSliceElementReadDirect(buf *bytes.Buffer, elemType types.Type,
elemAccess string) error {
+ // Handle basic types - read directly using their serializers (no flags)
+ if basic, ok := elemType.Underlying().(*types.Basic); ok {
+ switch basic.Kind() {
+ case types.Bool:
+ fmt.Fprintf(buf, "\t\t\t\t%s = buf.ReadBool()\n",
elemAccess)
+ case types.Int8:
+ fmt.Fprintf(buf, "\t\t\t\t%s =
int8(buf.ReadByte_())\n", elemAccess)
+ case types.Int16:
+ fmt.Fprintf(buf, "\t\t\t\t%s = buf.ReadInt16()\n",
elemAccess)
+ case types.Int32:
+ fmt.Fprintf(buf, "\t\t\t\t%s = buf.ReadVarint32()\n",
elemAccess)
+ case types.Int, types.Int64:
+ fmt.Fprintf(buf, "\t\t\t\t%s = buf.ReadVarint64()\n",
elemAccess)
+ case types.Uint8:
+ fmt.Fprintf(buf, "\t\t\t\t%s = buf.ReadByte_()\n",
elemAccess)
+ case types.Uint16:
+ fmt.Fprintf(buf, "\t\t\t\t%s =
uint16(buf.ReadInt16())\n", elemAccess)
+ case types.Uint32:
+ fmt.Fprintf(buf, "\t\t\t\t%s =
uint32(buf.ReadInt32())\n", elemAccess)
+ case types.Uint, types.Uint64:
+ fmt.Fprintf(buf, "\t\t\t\t%s =
uint64(buf.ReadInt64())\n", elemAccess)
+ case types.Float32:
+ fmt.Fprintf(buf, "\t\t\t\t%s = buf.ReadFloat32()\n",
elemAccess)
+ case types.Float64:
+ fmt.Fprintf(buf, "\t\t\t\t%s = buf.ReadFloat64()\n",
elemAccess)
+ case types.String:
+ // String serializer reads directly without flags
+ fmt.Fprintf(buf, "\t\t\t\t%s = fory.ReadString(buf)\n",
elemAccess)
+ default:
+ return fmt.Errorf("unsupported basic type for direct
element read: %s", basic.String())
+ }
+ return nil
+ }
+
+ return fmt.Errorf("unsupported element type for direct read: %s",
elemType.String())
+}
+
// generateMapReadInline generates inline map deserialization code following
the chunk-based format
func generateMapReadInline(buf *bytes.Buffer, mapType *types.Map, fieldAccess
string) error {
keyType := mapType.Key()
diff --git a/go/fory/codegen/encoder.go b/go/fory/codegen/encoder.go
index 804aac6d1..87f002439 100644
--- a/go/fory/codegen/encoder.go
+++ b/go/fory/codegen/encoder.go
@@ -137,7 +137,7 @@ func generateFieldWriteTyped(buf *bytes.Buffer, field
*FieldInfo) error {
fmt.Fprintf(buf, "\tbuf.WriteInt8(-1) //
NotNullValueFlag\n")
fmt.Fprintf(buf, "\tbuf.WriteFloat64(%s)\n",
fieldAccess)
case types.String:
- fmt.Fprintf(buf, "\tbuf.WriteInt8(0) // RefValueFlag\n")
+ fmt.Fprintf(buf, "\tbuf.WriteInt8(-1) //
NotNullValueFlag\n")
fmt.Fprintf(buf, "\tfory.WriteString(buf, %s)\n",
fieldAccess)
default:
fmt.Fprintf(buf, "\t// TODO: unsupported basic type
%s\n", basic.String())
@@ -160,10 +160,9 @@ func generateFieldWriteTyped(buf *bytes.Buffer, field
*FieldInfo) error {
fmt.Fprintf(buf, "\t\tbuf.WriteInt8(0) //
RefValueFlag\n")
fmt.Fprintf(buf, "\t\t// Write slice length\n")
fmt.Fprintf(buf,
"\t\tbuf.WriteVarUint32(uint32(len(%s)))\n", fieldAccess)
- fmt.Fprintf(buf, "\t\t// Write collection flags (13 =
NotDeclElementType + NotSameType + TrackingRef for dynamic slices)\n")
- fmt.Fprintf(buf, "\t\t// Always write collection flags
with tracking ref enabled (13)\n")
- fmt.Fprintf(buf, "\t\t// This matches the reflection
implementation which uses NewFory(true)\n")
- fmt.Fprintf(buf, "\t\tbuf.WriteInt8(13) // 12 + 1
(CollectionTrackingRef)\n")
+ fmt.Fprintf(buf, "\t\t// Write collection flags for
dynamic slice []interface{}\n")
+ fmt.Fprintf(buf, "\t\t// Only CollectionTrackingRef is
set (no declared type, may have different types)\n")
+ fmt.Fprintf(buf, "\t\tbuf.WriteInt8(1) //
CollectionTrackingRef only\n")
fmt.Fprintf(buf, "\t\t// Write each element using
WriteReferencable\n")
fmt.Fprintf(buf, "\t\tfor _, elem := range %s {\n",
fieldAccess)
fmt.Fprintf(buf, "\t\t\tf.WriteReferencable(buf,
reflect.ValueOf(elem))\n")
@@ -286,28 +285,19 @@ func generateSliceWriteInline(buf *bytes.Buffer,
sliceType *types.Slice, fieldAc
// Write collection header and elements for non-empty slice
fmt.Fprintf(buf, "\t\tif sliceLen > 0 {\n")
- // For codegen, follow reflection's behavior exactly:
- // Set CollectionNotDeclElementType (0b0100 = 4) and
CollectionNotSameType (0b1000 = 8)
- // Add CollectionTrackingRef (0b0001 = 1) when reference tracking is
enabled
- fmt.Fprintf(buf, "\t\t\tcollectFlag := 12 //
CollectionNotDeclElementType + CollectionNotSameType\n")
- fmt.Fprintf(buf, "\t\t\t// Access private field f.refTracking using
reflection to match behavior\n")
- fmt.Fprintf(buf, "\t\t\tforyValue := reflect.ValueOf(f).Elem()\n")
- fmt.Fprintf(buf, "\t\t\trefTrackingField :=
foryValue.FieldByName(\"refTracking\")\n")
- fmt.Fprintf(buf, "\t\t\tif refTrackingField.IsValid() &&
refTrackingField.Bool() {\n")
- fmt.Fprintf(buf, "\t\t\t\tcollectFlag |= 1 // Add
CollectionTrackingRef\n")
- fmt.Fprintf(buf, "\t\t\t}\n")
+ // For codegen, follow reflection's behavior:
+ // For typed slices, reflection only sets CollectionIsSameType (not
CollectionIsDeclElementType)
+ // because sliceSerializer.declaredType is nil
+ fmt.Fprintf(buf, "\t\t\tcollectFlag := 8 // CollectionIsSameType
only\n")
fmt.Fprintf(buf, "\t\t\tbuf.WriteInt8(int8(collectFlag))\n")
- // For each element, write type info + value (because
CollectionNotSameType is set)
- fmt.Fprintf(buf, "\t\t\tfor _, elem := range %s {\n", fieldAccess)
- fmt.Fprintf(buf, "\t\t\t\tbuf.WriteInt8(-1) // NotNullValueFlag\n")
-
- // Write element type ID
+ // Write element type ID since CollectionIsDeclElementType is not set
if err := generateElementTypeIDWriteInline(buf, elemType); err != nil {
return err
}
- // Write element value
+ // Write elements directly without per-element flags/type IDs
+ fmt.Fprintf(buf, "\t\t\tfor _, elem := range %s {\n", fieldAccess)
if err := generateSliceElementWriteInline(buf, elemType, "elem"); err
!= nil {
return err
}
diff --git a/go/fory/tests/generator_xlang_test.go
b/go/fory/tests/generator_xlang_test.go
index 3bc49711c..85704c60f 100644
--- a/go/fory/tests/generator_xlang_test.go
+++ b/go/fory/tests/generator_xlang_test.go
@@ -26,8 +26,8 @@ import (
"github.com/stretchr/testify/require"
)
-// TestActualCodegenName - Analyze actual type names used by codegen
-func TestActualCodegenName(t *testing.T) {
+// TestValidationDemoXlang - Test cross-language compatibility of
ValidationDemo
+func TestValidationDemoXlang(t *testing.T) {
// From source code analysis:
// RegisterSerializerFactory calculates: typeTag := pkgPath + "." +
typeName
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
