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

Stop pooling top-level objects #319

Merged
merged 1 commit into from
Nov 19, 2018
Merged

Conversation

axw
Copy link
Member

@axw axw commented Nov 19, 2018

Transaction, Span, and Error are no longer pooled.
Instead, new *Data types are introduced which are
pooled, and those are embedded within their related
top-level object types. We add a read-write mutex
to the Transaction and Span types to guard access
to those such that a concurrent StartSpan and
Transaction.End will not panic.

This change introduces an additional allocation for
each of StartTransaction, StartSpan, and NewError.
The time overhead (measured in nanoseconds) is not
enough to warrant living with the lack of safety.

It is now safe to call End() multiple times, and
to call StartSpan with an ended transaction or parent
span. When StartSpan is called with an ended
transaction, a dropped span will be created.

Closes #317

@codecov-io
Copy link

codecov-io commented Nov 19, 2018

Codecov Report

Merging #319 into master will increase coverage by 0.05%.
The diff coverage is 89.3%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #319      +/-   ##
==========================================
+ Coverage   80.82%   80.88%   +0.05%     
==========================================
  Files          96       96              
  Lines        5627     5701      +74     
==========================================
+ Hits         4548     4611      +63     
- Misses        826      828       +2     
- Partials      253      262       +9
Impacted Files Coverage Δ
gocontext.go 100% <100%> (ø) ⬆️
tracer.go 84.61% <100%> (ø) ⬆️
module/apmgocql/observer.go 95.91% <100%> (ø) ⬆️
error.go 74.56% <100%> (+2.41%) ⬆️
module/apmsql/conn.go 58.94% <100%> (ø) ⬆️
modelwriter.go 94.18% <100%> (ø) ⬆️
module/apmhttp/client.go 87.3% <100%> (ø) ⬆️
transaction.go 90.47% <81.81%> (-7.15%) ⬇️
span.go 85.93% <84.48%> (+1.72%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c589eb1...0545041. Read the comment docs.

Transaction, Span, and Error are no longer pooled.
Instead, new *Data types are introduced which are
pooled, and those are embedded within their related
top-level object types. We add a read-write mutex
to the Transaction and Span types to guard access
to those such that a concurrent StartSpan and
Transaction.End will not panic.

This change introduces an additional allocation for
each of StartTransaction, StartSpan, and NewError.
The time overhead (measured in nanoseconds) is not
enough to warrant living with the lack of safety.

It is now safe to call End() multiple times, and
to call StartSpan with an ended transaction or parent
span. When StartSpan is called with an ended
transaction, a dropped span will be created.
@axw
Copy link
Member Author

axw commented Nov 19, 2018

benchstat, with 10 runs of master & this branch:

name                                     old time/op    new time/op    delta
pkg:go.elastic.co/apm/internal/ringbuffer goos:linux goarch:amd64
Write-8                                     143ns ±29%     120ns ±18%  -16.26%  (p=0.012 n=10+10)
WriteBlockTo-8                             56.0ns ± 9%    55.5ns ±10%     ~     (p=0.631 n=10+10)
pkg:go.elastic.co/apm/internal/sqlutil goos:linux goarch:amd64
QuerySignature-8                           2.22µs ± 3%    2.45µs ± 9%  +10.21%  (p=0.000 n=9+10)
pkg:go.elastic.co/apm/internal/wildcard goos:linux goarch:amd64
WildcardMatcher-8                           691ns ±15%     694ns ± 8%     ~     (p=0.516 n=10+10)
RegexpMatcher-8                            12.6µs ±18%    11.9µs ±20%     ~     (p=0.113 n=10+9)
pkg:go.elastic.co/apm/model goos:linux goarch:amd64
MarshalTransactionFastJSON-8               1.39µs ±18%    1.36µs ± 4%     ~     (p=0.754 n=10+10)
MarshalTransactionStdlib-8                 9.60µs ± 9%    9.28µs ±10%     ~     (p=0.190 n=10+10)
pkg:go.elastic.co/apm/module/apmgin goos:linux goarch:amd64
WithoutMiddleware//hello/world-8            403ns ±18%     360ns ± 3%  -10.67%  (p=0.002 n=10+10)
WithoutMiddleware//sleep/1ms-8             1.13ms ± 1%    1.20ms ± 2%   +6.29%  (p=0.000 n=9+10)
WithMiddleware//hello/world-8              2.83µs ±26%    2.36µs ± 4%  -16.83%  (p=0.002 n=10+9)
WithMiddleware//sleep/1ms-8                1.20ms ± 1%    1.21ms ± 3%   +1.49%  (p=0.022 n=9+10)
pkg:go.elastic.co/apm/module/apmhttp goos:linux goarch:amd64
Client/baseline//hello/world-8             2.64ms ±40%    2.41ms ±41%     ~     (p=0.094 n=9+9)
Client/baseline//sleep/1ms-8               1.55ms ± 8%    1.64ms ± 1%   +5.35%  (p=0.004 n=9+9)
Client/wrapped//hello/world-8              2.65ms ±16%    2.19ms ±14%  -17.52%  (p=0.001 n=8+9)
Client/wrapped//sleep/1ms-8                1.62ms ± 4%    1.63ms ± 9%     ~     (p=0.315 n=9+10)
HandlerWithoutMiddleware//hello/world-8     526ns ± 8%     575ns ±17%   +9.29%  (p=0.003 n=10+10)
HandlerWithoutMiddleware//sleep/1ms-8      1.18ms ± 1%    1.18ms ± 4%     ~     (p=0.684 n=10+10)
HandlerWithMiddleware//hello/world-8       2.27µs ± 3%    2.49µs ± 5%   +9.72%  (p=0.000 n=9+9)
HandlerWithMiddleware//sleep/1ms-8         1.20ms ± 1%    1.20ms ± 3%     ~     (p=0.529 n=10+10)
pkg:go.elastic.co/apm/module/apmsql goos:linux goarch:amd64
StmtQueryContext/baseline-8                4.38µs ± 5%    4.87µs ±13%  +11.31%  (p=0.000 n=9+10)
StmtQueryContext/instrumented-8            5.63µs ± 9%    5.55µs ± 3%     ~     (p=0.447 n=10+9)

name                                     old speed      new speed      delta
pkg:go.elastic.co/apm/internal/ringbuffer goos:linux goarch:amd64
Write-8                                  7.32GB/s ±25%  8.64GB/s ±16%  +17.98%  (p=0.015 n=10+10)
WriteBlockTo-8                           5.37GB/s ± 9%  5.42GB/s ±10%     ~     (p=0.631 n=10+10)
pkg:go.elastic.co/apm/internal/sqlutil goos:linux goarch:amd64
QuerySignature-8                         54.4MB/s ± 3%  49.5MB/s ± 9%   -9.02%  (p=0.000 n=9+10)
pkg:go.elastic.co/apm/internal/wildcard goos:linux goarch:amd64
WildcardMatcher-8                        58.2MB/s ±14%  57.7MB/s ± 8%     ~     (p=0.481 n=10+10)
RegexpMatcher-8                          3.19MB/s ±16%  3.38MB/s ±17%     ~     (p=0.113 n=10+9)
pkg:go.elastic.co/apm/model goos:linux goarch:amd64
MarshalTransactionFastJSON-8              622MB/s ±15%   630MB/s ± 4%     ~     (p=0.739 n=10+10)
MarshalTransactionStdlib-8                134MB/s ±10%   139MB/s ± 9%     ~     (p=0.190 n=10+10)

name                                     old alloc/op   new alloc/op   delta
pkg:go.elastic.co/apm/internal/ringbuffer goos:linux goarch:amd64
Write-8                                     0.00B          0.00B          ~     (all equal)
WriteBlockTo-8                              0.00B          0.00B          ~     (all equal)
pkg:go.elastic.co/apm/internal/sqlutil goos:linux goarch:amd64
QuerySignature-8                            32.0B ± 0%     32.0B ± 0%     ~     (all equal)
pkg:go.elastic.co/apm/internal/wildcard goos:linux goarch:amd64
WildcardMatcher-8                           0.00B          0.00B          ~     (all equal)
RegexpMatcher-8                             0.00B          0.00B          ~     (all equal)
pkg:go.elastic.co/apm/model goos:linux goarch:amd64
MarshalTransactionFastJSON-8                16.0B ± 0%     16.0B ± 0%     ~     (all equal)
MarshalTransactionStdlib-8                   432B ± 0%      432B ± 0%     ~     (all equal)
pkg:go.elastic.co/apm/module/apmgin goos:linux goarch:amd64
WithoutMiddleware//hello/world-8             110B ± 4%      112B ± 0%     ~     (p=0.211 n=10+10)
WithoutMiddleware//sleep/1ms-8              0.00B          0.00B          ~     (all equal)
WithMiddleware//hello/world-8                444B ± 3%      474B ± 2%   +6.76%  (p=0.000 n=10+10)
WithMiddleware//sleep/1ms-8                  331B ± 4%      360B ± 3%   +8.83%  (p=0.000 n=9+9)
pkg:go.elastic.co/apm/module/apmhttp goos:linux goarch:amd64
Client/baseline//hello/world-8             18.1kB ± 3%    18.0kB ± 3%     ~     (p=0.315 n=10+10)
Client/baseline//sleep/1ms-8               4.93kB ± 0%    4.93kB ± 0%     ~     (p=0.754 n=10+10)
Client/wrapped//hello/world-8              21.2kB ±12%    20.9kB ±10%     ~     (p=1.000 n=10+10)
Client/wrapped//sleep/1ms-8                7.43kB ± 0%    7.60kB ± 0%   +2.32%  (p=0.000 n=9+10)
HandlerWithoutMiddleware//hello/world-8     74.0B ± 0%     78.2B ±13%     ~     (p=0.248 n=9+10)
HandlerWithoutMiddleware//sleep/1ms-8       0.00B          0.00B          ~     (all equal)
HandlerWithMiddleware//hello/world-8         463B ± 3%      484B ± 1%   +4.54%  (p=0.000 n=10+9)
HandlerWithMiddleware//sleep/1ms-8           376B ± 4%      409B ± 0%   +8.82%  (p=0.000 n=9+6)
pkg:go.elastic.co/apm/module/apmsql goos:linux goarch:amd64
StmtQueryContext/baseline-8                  528B ± 0%      848B ± 0%  +60.61%  (p=0.000 n=10+10)
StmtQueryContext/instrumented-8              602B ± 1%      630B ± 1%   +4.58%  (p=0.000 n=10+10)

name                                     old allocs/op  new allocs/op  delta
pkg:go.elastic.co/apm/internal/ringbuffer goos:linux goarch:amd64
Write-8                                      0.00           0.00          ~     (all equal)
WriteBlockTo-8                               0.00           0.00          ~     (all equal)
pkg:go.elastic.co/apm/internal/sqlutil goos:linux goarch:amd64
QuerySignature-8                             1.00 ± 0%      1.00 ± 0%     ~     (all equal)
pkg:go.elastic.co/apm/internal/wildcard goos:linux goarch:amd64
WildcardMatcher-8                            0.00           0.00          ~     (all equal)
RegexpMatcher-8                              0.00           0.00          ~     (all equal)
pkg:go.elastic.co/apm/model goos:linux goarch:amd64
MarshalTransactionFastJSON-8                 1.00 ± 0%      1.00 ± 0%     ~     (all equal)
MarshalTransactionStdlib-8                   7.00 ± 0%      7.00 ± 0%     ~     (all equal)
pkg:go.elastic.co/apm/module/apmgin goos:linux goarch:amd64
WithoutMiddleware//hello/world-8             3.00 ± 0%      3.00 ± 0%     ~     (all equal)
WithoutMiddleware//sleep/1ms-8               0.00           0.00          ~     (all equal)
WithMiddleware//hello/world-8                5.00 ± 0%      6.00 ± 0%  +20.00%  (p=0.000 n=10+10)
WithMiddleware//sleep/1ms-8                  2.60 ±54%      3.00 ± 0%     ~     (p=0.059 n=10+8)
pkg:go.elastic.co/apm/module/apmhttp goos:linux goarch:amd64
Client/baseline//hello/world-8                133 ± 1%       133 ± 1%     ~     (p=1.000 n=8+8)
Client/baseline//sleep/1ms-8                 60.0 ± 0%      60.0 ± 0%     ~     (all equal)
Client/wrapped//hello/world-8                 147 ± 4%       148 ± 3%     ~     (p=0.834 n=10+10)
Client/wrapped//sleep/1ms-8                  74.0 ± 0%      75.0 ± 0%   +1.35%  (p=0.000 n=10+10)
HandlerWithoutMiddleware//hello/world-8      3.00 ± 0%      3.00 ± 0%     ~     (all equal)
HandlerWithoutMiddleware//sleep/1ms-8        0.00           0.00          ~     (all equal)
HandlerWithMiddleware//hello/world-8         7.00 ± 0%      8.00 ± 0%  +14.29%  (p=0.000 n=10+10)
HandlerWithMiddleware//sleep/1ms-8           4.50 ±33%      5.33 ±12%  +18.52%  (p=0.010 n=10+9)
pkg:go.elastic.co/apm/module/apmsql goos:linux goarch:amd64
StmtQueryContext/baseline-8                  8.00 ± 0%     10.00 ± 0%  +25.00%  (p=0.000 n=10+10)
StmtQueryContext/instrumented-8              9.00 ± 0%     10.00 ± 0%  +11.11%  (p=0.000 n=10+10)

@axw axw merged commit b692d0f into elastic:master Nov 19, 2018
@axw axw deleted the internal-transaction branch November 19, 2018 06:34
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.

2 participants