Skip to content

Commit

Permalink
refactor: correcting the cache usage (#23)
Browse files Browse the repository at this point in the history
  • Loading branch information
tiendc authored Nov 23, 2024
1 parent 747dea2 commit 4952d9c
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 112 deletions.
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
linters-settings:
funlen:
lines: 100
lines: 120
statements: 80
gci:
sections:
Expand Down
6 changes: 6 additions & 0 deletions base_copier.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ func (c *nopCopier) Copy(dst, src reflect.Value) error {
return nil
}

var defaultNopCopier = &nopCopier{}

// value2PtrCopier data structure of copier that copies from a value to a pointer
type value2PtrCopier struct {
ctx *Context
Expand Down Expand Up @@ -93,6 +95,8 @@ func (c *directCopier) Copy(dst, src reflect.Value) error {
return nil
}

var defaultDirectCopier = &directCopier{}

// convCopier copier that does copying with converting `src` value to `dst` type
type convCopier struct {
}
Expand All @@ -101,3 +105,5 @@ func (c *convCopier) Copy(dst, src reflect.Value) error {
dst.Set(src.Convert(dst.Type()))
return nil
}

var defaultConvCopier = &convCopier{}
113 changes: 63 additions & 50 deletions build_copier.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,96 +98,109 @@ func defaultContext() *Context {
// buildCopier build copier for handling copy from `srcType` to `dstType`
//
//nolint:gocognit,gocyclo
func buildCopier(ctx *Context, dstType, srcType reflect.Type) (copier, error) {
func buildCopier(ctx *Context, dstType, srcType reflect.Type) (copier copier, err error) {
// Finds cached copier, returns it if found
cacheKey := ctx.createCacheKey(dstType, srcType)
ctx.mu.RLock()
cp, ok := ctx.copierCacheMap[*cacheKey]
ctx.mu.RUnlock()
if ok {
return cp, nil
}

dstKind, srcKind := dstType.Kind(), srcType.Kind()

// Trivial case
if simpleKindMask&(1<<srcKind) > 0 {
if dstType == srcType {
copier = defaultDirectCopier
goto OnComplete
}
if srcType.ConvertibleTo(dstType) {
copier = defaultConvCopier
goto OnComplete
}
}

if dstKind == reflect.Interface {
return &toIfaceCopier{ctx: ctx}, nil
cp := &toIfaceCopier{ctx: ctx}
copier, err = cp, cp.init(dstType, srcType)
goto OnComplete
}
if srcKind == reflect.Interface {
return &fromIfaceCopier{ctx: ctx}, nil
cp := &fromIfaceCopier{ctx: ctx}
copier, err = cp, cp.init(dstType, srcType)
goto OnComplete
}

//nolint:nestif
if srcKind == reflect.Pointer {
if dstKind == reflect.Pointer { // ptr -> ptr
copier := &ptr2PtrCopier{ctx: ctx}
if err := copier.init(dstType, srcType); err != nil {
return nil, err
}
return copier, nil
cp := &ptr2PtrCopier{ctx: ctx}
copier, err = cp, cp.init(dstType, srcType)
goto OnComplete
} else { // ptr -> value
if !ctx.CopyBetweenPtrAndValue {
return onNonCopyable(ctx, dstType, srcType)
}
copier := &ptr2ValueCopier{ctx: ctx}
if err := copier.init(dstType, srcType); err != nil {
return nil, err
goto OnNonCopyable
}
return copier, nil
cp := &ptr2ValueCopier{ctx: ctx}
copier, err = cp, cp.init(dstType, srcType)
goto OnComplete
}
} else {
if dstKind == reflect.Pointer { // value -> ptr
if !ctx.CopyBetweenPtrAndValue {
return onNonCopyable(ctx, dstType, srcType)
goto OnNonCopyable
}
copier := &value2PtrCopier{ctx: ctx}
if err := copier.init(dstType, srcType); err != nil {
return nil, err
}
return copier, nil
cp := &value2PtrCopier{ctx: ctx}
copier, err = cp, cp.init(dstType, srcType)
goto OnComplete
}
}

// Both are not Pointers
if srcKind == reflect.Slice || srcKind == reflect.Array {
if dstKind != reflect.Slice && dstKind != reflect.Array {
return onNonCopyable(ctx, dstType, srcType)
}
copier := &sliceCopier{ctx: ctx}
if err := copier.init(dstType, srcType); err != nil {
return nil, err
goto OnNonCopyable
}
return copier, nil
cp := &sliceCopier{ctx: ctx}
copier, err = cp, cp.init(dstType, srcType)
goto OnComplete
}

if srcKind == reflect.Struct {
if dstKind != reflect.Struct {
return onNonCopyable(ctx, dstType, srcType)
}
copier := &structCopier{ctx: ctx}
if err := copier.init(dstType, srcType); err != nil {
return nil, err
goto OnNonCopyable
}
return copier, nil
cp := &structCopier{ctx: ctx}
copier, err = cp, cp.init(dstType, srcType)
goto OnComplete
}

if srcKind == reflect.Map {
if dstKind != reflect.Map {
return onNonCopyable(ctx, dstType, srcType)
goto OnNonCopyable
}
copier := &mapCopier{ctx: ctx}
if err := copier.init(dstType, srcType); err != nil {
return nil, err
}
return copier, nil
cp := &mapCopier{ctx: ctx}
copier, err = cp, cp.init(dstType, srcType)
goto OnComplete
}

// Trivial case
if simpleKindMask&(1<<srcKind) > 0 {
if dstType == srcType {
return &directCopier{}, nil
}
if srcType.ConvertibleTo(dstType) {
return &convCopier{}, nil
OnComplete:
if err == nil {
if copier != nil {
ctx.mu.Lock()
ctx.copierCacheMap[*cacheKey] = copier
ctx.mu.Unlock()
return copier, err
}
} else {
return nil, err
}

return onNonCopyable(ctx, dstType, srcType)
}

func onNonCopyable(ctx *Context, dstType, srcType reflect.Type) (copier, error) {
OnNonCopyable:
if ctx.IgnoreNonCopyableTypes {
return &nopCopier{}, nil
return defaultNopCopier, nil
}
return nil, fmt.Errorf("%w: %v -> %v", ErrTypeNonCopyable, srcType, dstType)
}
16 changes: 0 additions & 16 deletions deepcopy.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,26 +92,10 @@ func Copy(dst, src any, options ...Option) (err error) {
}
ctx.prepare()

cacheKey := ctx.createCacheKey(dstType, srcType)
if ctx.UseGlobalCache {
ctx.mu.RLock()
cp, ok := ctx.copierCacheMap[*cacheKey]
ctx.mu.RUnlock()
if ok {
return cp.Copy(dstVal, srcVal)
}
}

cp, err := buildCopier(ctx, dstType, srcType)
if err != nil {
return err
}
if ctx.UseGlobalCache {
ctx.mu.Lock()
ctx.copierCacheMap[*cacheKey] = cp
ctx.mu.Unlock()
}

return cp.Copy(dstVal, srcVal)
}

Expand Down
8 changes: 8 additions & 0 deletions iface_copier.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ type fromIfaceCopier struct {
ctx *Context
}

func (c *fromIfaceCopier) init(dst, src reflect.Type) error {
return nil
}

// Copy implementation of Copy function for from-iface copier
func (c *fromIfaceCopier) Copy(dst, src reflect.Value) error {
for src.Kind() == reflect.Interface {
Expand All @@ -30,6 +34,10 @@ type toIfaceCopier struct {
ctx *Context
}

func (c *toIfaceCopier) init(dst, src reflect.Type) error {
return nil
}

// Copy implementation of Copy function for to-iface copier
func (c *toIfaceCopier) Copy(dst, src reflect.Value) error {
for src.Kind() == reflect.Interface {
Expand Down
8 changes: 4 additions & 4 deletions map_copier.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,24 +44,24 @@ func (c *mapCopier) init(dstType, srcType reflect.Type) error {
dstKeyType, dstValType := dstType.Key(), dstType.Elem()
buildKeyCopier, buildValCopier := true, true

// OPTIMIZATION: buildCopier() can handle this nicely, but it will add another wrapping layer
// OPTIMIZATION: buildCopier() can handle this nicely
if simpleKindMask&(1<<srcKeyType.Kind()) > 0 {
if srcKeyType == dstKeyType {
// Just keep c.keyCopier = nil
buildKeyCopier = false
} else if srcKeyType.ConvertibleTo(dstKeyType) {
c.keyCopier = &mapItemCopier{dstType: dstKeyType, copier: &convCopier{}}
c.keyCopier = &mapItemCopier{dstType: dstKeyType, copier: defaultConvCopier}
buildKeyCopier = false
}
}

// OPTIMIZATION: buildCopier() can handle this nicely, but it will add another wrapping layer
// OPTIMIZATION: buildCopier() can handle this nicely
if simpleKindMask&(1<<srcValType.Kind()) > 0 {
if srcValType == dstValType {
// Just keep c.valueCopier = nil
buildValCopier = false
} else if srcValType.ConvertibleTo(dstValType) {
c.valueCopier = &mapItemCopier{dstType: dstValType, copier: &convCopier{}}
c.valueCopier = &mapItemCopier{dstType: dstValType, copier: defaultConvCopier}
buildValCopier = false
}
}
Expand Down
28 changes: 1 addition & 27 deletions slice_copier.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,32 +48,6 @@ func (c *sliceCopier) Copy(dst, src reflect.Value) error {
}

func (c *sliceCopier) init(dstType, srcType reflect.Type) (err error) {
dstType, srcType = dstType.Elem(), srcType.Elem()
srcKind := srcType.Kind()

// OPTIMIZATION: buildCopier() can handle this nicely, but it will add another wrapping layer
if simpleKindMask&(1<<srcKind) > 0 {
if srcType == dstType {
c.itemCopier = &directCopier{}
return nil
}
if srcType.ConvertibleTo(dstType) {
c.itemCopier = &convCopier{}
return nil
}
}

// OPTIMIZATION: buildCopier() can handle this nicely, but it will add another wrapping layer
if srcKind == reflect.Struct {
c.ctx.mu.RLock()
cp, ok := c.ctx.copierCacheMap[*c.ctx.createCacheKey(dstType, srcType)]
c.ctx.mu.RUnlock()
if ok {
c.itemCopier = cp
return nil
}
}

c.itemCopier, err = buildCopier(c.ctx, dstType, srcType)
c.itemCopier, err = buildCopier(c.ctx, dstType.Elem(), srcType.Elem())
return
}
16 changes: 2 additions & 14 deletions struct_copier.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,6 @@ func (c *structCopier) Copy(dst, src reflect.Value) error {

//nolint:gocognit,gocyclo
func (c *structCopier) init(dstType, srcType reflect.Type) (err error) {
cacheKey := c.ctx.createCacheKey(dstType, srcType)
c.ctx.mu.RLock()
cp, ok := c.ctx.copierCacheMap[*cacheKey]
c.ctx.mu.RUnlock()
if ok {
c.fieldCopiers = cp.(*structCopier).fieldCopiers //nolint:forcetypeassert
return nil
}

var dstCopyingMethods map[string]*reflect.Method
if c.ctx.CopyBetweenStructFieldAndMethod {
dstCopyingMethods = c.parseCopyingMethods(dstType)
Expand Down Expand Up @@ -109,9 +100,6 @@ func (c *structCopier) init(dstType, srcType reflect.Type) (err error) {
}
}

c.ctx.mu.Lock()
c.ctx.copierCacheMap[*cacheKey] = c
c.ctx.mu.Unlock()
return nil
}

Expand Down Expand Up @@ -206,15 +194,15 @@ func (c *structCopier) parseAllNestedFields(typ reflect.Type, index []int) map[s
func (c *structCopier) buildCopier(dstType, srcType reflect.Type, dstDetail, srcDetail *fieldDetail) (copier, error) {
df, sf := dstDetail.field, srcDetail.field

// OPTIMIZATION: buildCopier() can handle this nicely, but it will add another wrapping layer
// OPTIMIZATION: buildCopier() can handle this nicely
if simpleKindMask&(1<<sf.Type.Kind()) > 0 {
if sf.Type == df.Type {
// NOTE: pass nil to unset custom copier and trigger direct copying.
// We can pass `&directCopier{}` for the same result (but it's a bit slower).
return c.createField2FieldCopier(dstDetail, srcDetail, nil), nil
}
if sf.Type.ConvertibleTo(df.Type) {
return c.createField2FieldCopier(dstDetail, srcDetail, &convCopier{}), nil
return c.createField2FieldCopier(dstDetail, srcDetail, defaultConvCopier), nil
}
}

Expand Down

0 comments on commit 4952d9c

Please sign in to comment.