Skip to content

Commit

Permalink
fix: fix native C FSM issues (#25)
Browse files Browse the repository at this point in the history
* add submodule code

* build: update asm2asm

* fix: incorrect free size of `ReqCache`

* update bench.py

* opt: reduce J2TExtra memory

* test: add `go:nocheckptr`

* test: add parallel tests

* feat: update pcsp

* feat: update asm2asm

* update sonic

* not support go1.15

* fallback for all

* update sonic

* rollback asm2asm

* update sonic

* update go mod
  • Loading branch information
AsterDY authored May 8, 2023
1 parent 1f846f2 commit 683f982
Show file tree
Hide file tree
Showing 34 changed files with 5,896 additions and 6,512 deletions.
33 changes: 0 additions & 33 deletions .github/workflows/push-check-linux-arm.yml

This file was deleted.

34 changes: 34 additions & 0 deletions .github/workflows/push-check-linux-compat.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
name: Compatibility Test

on: push

jobs:
build:
strategy:
matrix:
go-version: [1.15.x, 1.20.x]
os: [ARM64, X64]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v2

- name: Set up Go
uses: actions/setup-go@v2
with:
go-version: ${{ matrix.go-version }}

- uses: actions/cache@v2
with:
path: ~/go/pkg/mod
key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
restore-keys: |
${{ runner.os }}-go-
- name: Unit Test
run: |
go test -race github.com/cloudwego/dynamicgo/thrift
go test -race github.com/cloudwego/dynamicgo/thrift/annotation
go test -race github.com/cloudwego/dynamicgo/thrift/generic
go test -race github.com/cloudwego/dynamicgo/conv/t2j
go test -race github.com/cloudwego/dynamicgo/http
go test -race github.com/cloudwego/dynamicgo/internal/json
2 changes: 1 addition & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
[submodule "tools/asm2asm"]
[submodule "asm2asm"]
path = tools/asm2asm
url = https://github.com/chenzhuoyu/asm2asm.git
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,4 @@ $(foreach \
arch, \
${ARCH}, \
$(eval $(call build_arch,${arch})) \
)
)
2 changes: 1 addition & 1 deletion bench.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import argparse

gbench_prefix = "SONIC_NO_ASYNC_GC=1 go test -benchmem -run=none "
mainbranch = "master"
mainbranch = "main"

def run(cmd):
print(cmd)
Expand Down
20 changes: 13 additions & 7 deletions conv/j2t/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/cloudwego/dynamicgo/http"
"github.com/cloudwego/dynamicgo/internal/json"
"github.com/cloudwego/dynamicgo/internal/native/types"
"github.com/cloudwego/dynamicgo/internal/rt"
"github.com/cloudwego/dynamicgo/meta"
"github.com/cloudwego/dynamicgo/thrift"
)
Expand All @@ -37,15 +38,20 @@ func newError(code meta.ErrCode, msg string, err error) error {

type _J2TExtra_STRUCT struct {
desc unsafe.Pointer
reqs thrift.RequiresBitmap
reqs string
}

func getJ2TExtraStruct(fsm *types.J2TStateMachine, offset int) (*thrift.TypeDescriptor, *_J2TExtra_STRUCT) {
//go:nocheckptr
func getJ2TExtraStruct(fsm *types.J2TStateMachine, offset int) (td *thrift.TypeDescriptor, reqs thrift.RequiresBitmap) {
state := fsm.At(offset - 1)
if state == nil {
return nil, nil
return nil, thrift.RequiresBitmap{}
}
return (*thrift.TypeDescriptor)(state.TdPointer()), (*_J2TExtra_STRUCT)(unsafe.Pointer(&state.Extra))
td = (*thrift.TypeDescriptor)(unsafe.Pointer(state.TdPointer()))
je := (*_J2TExtra_STRUCT)(unsafe.Pointer(&state.Extra))
v := rt.Str2Mem(je.reqs)
reqs = *(*thrift.RequiresBitmap)(unsafe.Pointer(&v))
return
}

func (self BinaryConv) handleError(ctx context.Context, fsm *types.J2TStateMachine, buf *[]byte, src []byte, req http.RequestGetter, ret uint64, top bool) (cont bool, err error) {
Expand All @@ -55,14 +61,14 @@ func (self BinaryConv) handleError(ctx context.Context, fsm *types.J2TStateMachi
switch e {
case types.ERR_HTTP_MAPPING:
{
desc, ext := getJ2TExtraStruct(fsm, fsm.SP)
if desc == nil || ext == nil {
desc, reqs := getJ2TExtraStruct(fsm, fsm.SP)
if desc == nil {
return false, newError(meta.ErrConvert, "invalid json input", nil)
}
if desc.Type() != thrift.STRUCT {
return false, newError(meta.ErrConvert, "invalid descriptor while http mapping", nil)
}
return true, self.writeHttpRequestToThrift(ctx, req, desc.Struct(), &ext.reqs, buf, false, top)
return true, self.writeHttpRequestToThrift(ctx, req, desc.Struct(), reqs, buf, false, top)
}
case types.ERR_HTTP_MAPPING_END:
{
Expand Down
11 changes: 9 additions & 2 deletions conv/j2t/impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,14 @@ import (
"github.com/cloudwego/dynamicgo/thrift/base"
)

const (
_GUARD_SLICE_FACTOR = 1
)

func (self *BinaryConv) do(ctx context.Context, src []byte, desc *thrift.TypeDescriptor, buf *[]byte, req http.RequestGetter) error {
//NOTICE: output buffer must be larger than src buffer
rt.GuardSlice(buf, len(src)*_GUARD_SLICE_FACTOR)

if self.opts.EnableThriftBase {
if f := desc.Struct().GetRequestBase(); f != nil {
if err := writeRequestBaseToThrift(ctx, buf, f); err != nil {
Expand All @@ -50,7 +57,7 @@ func (self *BinaryConv) do(ctx context.Context, src []byte, desc *thrift.TypeDes
st.Requires().CopyTo(reqs)
// check if any http-mapping exists
if desc.Struct().HttpMappingFields() != nil {
if err := self.writeHttpRequestToThrift(ctx, req, st, reqs, buf, true, true); err != nil {
if err := self.writeHttpRequestToThrift(ctx, req, st, *reqs, buf, true, true); err != nil {
return err
}
}
Expand Down Expand Up @@ -175,7 +182,7 @@ func writeRequestBaseToThrift(ctx context.Context, buf *[]byte, field *thrift.Fi
return nil
}

func (self *BinaryConv) writeHttpRequestToThrift(ctx context.Context, req http.RequestGetter, desc *thrift.StructDescriptor, reqs *thrift.RequiresBitmap, buf *[]byte, nobody bool, top bool) (err error) {
func (self *BinaryConv) writeHttpRequestToThrift(ctx context.Context, req http.RequestGetter, desc *thrift.StructDescriptor, reqs thrift.RequiresBitmap, buf *[]byte, nobody bool, top bool) (err error) {
if req == nil {
return newError(meta.ErrInvalidParam, "http request is nil", nil)
}
Expand Down
4 changes: 2 additions & 2 deletions conv/t2j/conv_amd64_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//go:build amd64
// +build amd64
//go:build amd64 && go1.16
// +build amd64,go1.16

/**
* Copyright 2023 CloudWeGo Authors.
Expand Down
4 changes: 2 additions & 2 deletions conv/t2j/conv_fallback_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//go:build !amd64
// +build !amd64
//go:build !amd64 || !go1.16
// +build !amd64 !go1.16

/**
* Copyright 2023 CloudWeGo Authors.
Expand Down
7 changes: 7 additions & 0 deletions conv/t2j/impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ import (
"github.com/cloudwego/dynamicgo/thrift/base"
)

const (
_GUARD_SLICE_FACTOR = 2
)

//go:noinline
func wrapError(code meta.ErrCode, msg string, err error) error {
// panic(msg)
Expand Down Expand Up @@ -66,6 +70,9 @@ func (self *BinaryConv) readResponseBase(ctx context.Context, p *thrift.BinaryPr
}

func (self *BinaryConv) do(ctx context.Context, src []byte, desc *thrift.TypeDescriptor, out *[]byte, resp http.ResponseSetter) (err error) {
//NOTICE: output buffer must be larger than src buffer
rt.GuardSlice(out, len(src)*_GUARD_SLICE_FACTOR)

var p = thrift.BinaryProtocol{
Buf: src,
}
Expand Down
9 changes: 1 addition & 8 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,17 @@ go 1.16

require (
github.com/apache/thrift v0.13.0
github.com/bytedance/sonic v1.8.3-0.20230228111319-6d60889e3b65
github.com/bytedance/sonic v1.8.8
github.com/chenzhuoyu/base64x v0.0.0-20221115062448-fe3a3abad311
github.com/choleraehyq/pid v0.0.16 // indirect
github.com/cloudwego/kitex v0.4.4
github.com/cloudwego/thriftgo v0.2.7
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
github.com/google/go-cmp v0.5.9 // indirect
github.com/iancoleman/strcase v0.2.0
github.com/klauspost/cpuid/v2 v2.2.4
github.com/kr/text v0.2.0 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/stretchr/testify v1.8.2
github.com/thrift-iterator/go v0.0.0-20190402154806-9b5a67519118
github.com/v2pro/plz v0.0.0-20221028024117-e5f9aec5b631 // indirect
github.com/v2pro/quokka v0.0.0-20171201153428-382cb39c6ee6 // indirect
github.com/v2pro/wombat v0.0.0-20180402055224-a56dbdcddef2 // indirect
golang.org/x/arch v0.2.0 // indirect
golang.org/x/text v0.7.0 // indirect
google.golang.org/genproto v0.0.0-20220916172020-2692e8806bfa // indirect
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
)
Loading

0 comments on commit 683f982

Please sign in to comment.