Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zap.Any: Reduce stack size with generics #1310

Merged
merged 1 commit into from Aug 1, 2023

Commits on Aug 1, 2023

  1. zap.Any: Reduce stack size

    Yet another attempt at reducing the stack size of zap.Any,
    borrowing from uber-go#1301, uber-go#1303, uber-go#1304, uber-go#1305, uber-go#1307, and 1308.
    
    This approach defines a generic data type for field constructors
    of a specific type. This is similar to the lookup map in uber-go#1307,
    minus the map lookup, the interface match, or reflection.
    
        type anyFieldC[T any] func(string, T) Field
    
    The generic data type provides a non-generic method
    matching the interface:
    
        interface{ Any(string, any) Field }
    
    Stack size:
    The stack size of zap.Any following this change is 0xc0.
    
        % go build -gcflags -S 2>&1 | grep ^go.uber.org/zap.Any
        go.uber.org/zap.Any STEXT size=5861 args=0x20 locals=0xc0 funcid=0x0 align=0x0
    
    This is just 8 bytes more than uber-go#1305,
    which is the smallest stack size of all other attempts.
    
    Allocations:
    Everything appears to get inlined with no heap escapes:
    
        go build -gcflags -m 2>&1 |
          grep field.go |
          perl -n -e 'next unless m{^./field.go:(\d+)}; print if ($1 >= 413)' |
          grep 'escapes'
    
    (Line 413 declares anyFieldC)
    
    Besides that, the output of `-m` for the relevant section of code
    consists of almost entirely:
    
        ./field.go:415:6: can inline anyFieldC[go.shape.bool].Any
        ./field.go:415:6: can inline anyFieldC[go.shape.[]bool].Any
        ./field.go:415:6: can inline anyFieldC[go.shape.complex128].Any
        [...]
        ./field.go:415:6: inlining call to anyFieldC[go.shape.complex128].Any
        ./field.go:415:6: inlining call to anyFieldC[go.shape.[]bool].Any
        ./field.go:415:6: inlining call to anyFieldC[go.shape.bool].Any
    
    Followed by:
    
        ./field.go:428:10: leaking param: key
        ./field.go:428:22: leaking param: value
    
    Maintainability:
    Unlike some of the other approaches, this variant is more maintainable.
    The `zap.Any` function looks roughly the same.
    Adding new branches there is obvious, and requires no duplication.
    
    Performance:
    This is a net improvement on all BenchmarkAny calls
    except "any-no-logger" which calls `zap.Any` and discards the result.
    
    ```
    name                        old time/op    new time/op    delta
    Any/str-no-logger-2           8.77ns ± 0%    8.75ns ± 1%     ~     (p=0.159 n=4+5)
    Any/any-no-logger-2           54.1ns ± 0%    81.6ns ± 0%  +50.71%  (p=0.016 n=5+4)
    Any/str-with-logger-2         1.38µs ± 3%    1.38µs ± 4%     ~     (p=0.841 n=5+5)
    Any/any-with-logger-2         1.60µs ±22%    1.37µs ± 1%     ~     (p=0.151 n=5+5)
    Any/str-in-go-2               3.41µs ± 1%    3.42µs ± 5%     ~     (p=0.905 n=4+5)
    Any/any-in-go-2               5.98µs ± 1%    3.68µs ± 6%  -38.44%  (p=0.008 n=5+5)
    Any/str-in-go-with-stack-2    3.42µs ± 2%    3.46µs ± 3%     ~     (p=0.421 n=5+5)
    Any/any-in-go-with-stack-2    5.98µs ± 3%    3.65µs ± 3%  -38.95%  (p=0.008 n=5+5)
    
    name                        old alloc/op   new alloc/op   delta
    Any/str-no-logger-2            0.00B          0.00B          ~     (all equal)
    Any/any-no-logger-2            0.00B          0.00B          ~     (all equal)
    Any/str-with-logger-2          64.0B ± 0%     64.0B ± 0%     ~     (all equal)
    Any/any-with-logger-2          64.0B ± 0%     64.0B ± 0%     ~     (all equal)
    Any/str-in-go-2                88.5B ± 1%     88.0B ± 0%     ~     (p=0.429 n=4+4)
    Any/any-in-go-2                88.0B ± 0%     88.0B ± 0%     ~     (all equal)
    Any/str-in-go-with-stack-2     88.0B ± 0%     88.0B ± 0%     ~     (all equal)
    Any/any-in-go-with-stack-2     88.0B ± 0%     88.0B ± 0%     ~     (all equal)
    
    name                        old allocs/op  new allocs/op  delta
    Any/str-no-logger-2             0.00           0.00          ~     (all equal)
    Any/any-no-logger-2             0.00           0.00          ~     (all equal)
    Any/str-with-logger-2           1.00 ± 0%      1.00 ± 0%     ~     (all equal)
    Any/any-with-logger-2           1.00 ± 0%      1.00 ± 0%     ~     (all equal)
    Any/str-in-go-2                 2.00 ± 0%      2.00 ± 0%     ~     (all equal)
    Any/any-in-go-2                 2.00 ± 0%      2.00 ± 0%     ~     (all equal)
    Any/str-in-go-with-stack-2      2.00 ± 0%      2.00 ± 0%     ~     (all equal)
    Any/any-in-go-with-stack-2      2.00 ± 0%      2.00 ± 0%     ~     (all equal)
    ```
    
    I believe this is acceptable because that's not a real use case;
    we expect the result to be used with a logger.
    abhinav committed Aug 1, 2023
    Configuration menu
    Copy the full SHA
    66c34c2 View commit details
    Browse the repository at this point in the history