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.

Sign up for GitHub

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

Jump to bottom

Remove fmt call in the session for each call #1680

Merged
merged 2 commits into from
May 9, 2018
Merged

Remove fmt call in the session for each call #1680

merged 2 commits into from
May 9, 2018

Conversation

joshblakeley
Copy link
Member

@joshblakeley joshblakeley commented May 8, 2018 •
edited
Loading

Fixes #1632

Running gateway headless with auth-key.

Before change:

Showing top 10 nodes out of 182
      flat  flat%   sum%        cum   cum%
     0.04s  0.58%  0.58%      0.04s  0.58%  fmt.(*fmt).fmt_sbx
     0.04s  0.58%  1.16%      0.06s  0.87%  github.com/TykTechnologies/tyk/vendor/github.com/TykTechnologies/redigocluster/rediscluster.ConcurrentMap.Iter.func1.1
     0.02s  0.29%  1.45%      0.04s  0.58%  net/textproto.CanonicalMIMEHeaderKey
     0.02s  0.29%  1.74%      0.02s  0.29%  net/textproto.validHeaderFieldByte (inline)
     0.02s  0.29%  2.03%      0.02s  0.29%  strings.TrimLeftFunc
     0.01s  0.14%  2.17%      0.01s  0.14%  bufio.(*Reader).bufio.reset
     0.01s  0.14%  2.32%      0.01s  0.14%  bufio.(*Writer).WriteString
     0.01s  0.14%  2.46%      0.01s  0.14%  bytes.(*Buffer).grow
     0.01s  0.14%  2.61%      0.01s  0.14%  bytes.(*Buffer).tryGrowByReslice
     0.01s  0.14%  2.75%      0.01s  0.14%  encoding/json.(*arrayEncoder).encode

After:

Showing top 10 nodes out of 153
      flat  flat%   sum%        cum   cum%
     0.02s   0.3%   0.3%      0.02s   0.3%  bytes.(*Buffer).WriteString
     0.02s   0.3%  0.61%      0.02s   0.3%  context.(*cancelCtx).cancel
     0.02s   0.3%  0.91%      0.03s  0.46%  context.(*valueCtx).Value
     0.02s   0.3%  1.22%      0.02s   0.3%  github.com/TykTechnologies/tyk/config.Global (inline)
     0.02s   0.3%  1.52%      0.20s  3.04%  main.createMiddleware.func1.1
     0.02s   0.3%  1.82%      0.02s   0.3%  net/textproto.CanonicalMIMEHeaderKey
     0.02s   0.3%  2.13%      0.02s   0.3%  sync.(*RWMutex).RLock
     0.01s  0.15%  2.28%      0.01s  0.15%  bytes.(*Buffer).Write
     0.01s  0.15%  2.43%      0.01s  0.15%  bytes.(*Buffer).tryGrowByReslice (inline)
     0.01s  0.15%  2.58%      0.01s  0.15%  context.(*emptyCtx).Value

@joshblakeley
Copy link
Member Author

So this change is super small...

@buger @dencoded For MW where a similar change needs to be made should that be a new ticket each time or should i just exand this one with other changes?

@joshblakeley joshblakeley self-assigned this May 8, 2018
@buger
Copy link
Member

buger commented May 8, 2018

Do you think it makes sense instead of comparing profiling information, actually compare benchmarks output?

And pls try using https://godoc.org/golang.org/x/tools/cmd/benchcmp for this

@joshblakeley
Copy link
Member Author

Ok will try that.

@dencoded
Copy link
Contributor

dencoded commented May 8, 2018 •
edited
Loading

Thats a pretty minor change but it could contribute to other code performance improvement, where this function is used. However if you compare benchmarks for some MW this improvement might be "diluted" as this function could be a small fraction of lots of logic running under the MW's hood.

So, for that kind of changes I would create dedicated benchmark to do more granular performance analysis - i.e. create benchmark for func (s *SessionState) Hash() string to prove that this piece of code is faster now (past benchcmp output) as Leon suggested. This function is actually doing two heavy things - user session state encoding and jungling with string so I think it is worth to have bench mark for it anyways (say we switch from message pack to something else and it becomes x5 time faster!)

@joshblakeley
Copy link
Member Author

joshblakeley commented May 8, 2018 •
edited
Loading 

benchmark                                                   old ns/op     new ns/op     delta
BenchmarkURLReplacer-4                                      14164         14388         +1.58%
BenchmarkTagHeaders-4                                       1263          809           -35.95%
BenchmarkDefaultVersion-4                                   1497296       1498034       +0.05%
BenchmarkApiReload-4                                        4120261       4204786       +2.05%
BenchmarkMultiSession_BA_Standard_OK-4                      1417533       1831204       +29.18%
BenchmarkBearerTokenAuthKeySession-4                        1516428       1537190       +1.37%
BenchmarkMultiAuthBackwardsCompatibleSession-4              1559793       1537316       -1.44%
BenchmarkBasicAuth-4                                        134240485     134034371     -0.15%
BenchmarkContextVarsMiddleware-4                            1314566       1303267       -0.86%
BenchmarkMiddlewareContextVars_ProcessRequest_cookies-4     228473        240776        +5.38%
BenchmarkHMACAuthSessionPass-4                              1307875       1311953       +0.31%
BenchmarkHMACAuthSessionPassWithHeaderField-4               1324638       1317227       -0.56%
BenchmarkIPBlacklistMiddleware-4                            27253         30617         +12.34%
BenchmarkIPMiddlewarePass-4                                 17405         17182         -1.28%
BenchmarkJWTSessionHMAC-4                                   638267        624363        -2.18%
BenchmarkJWTSessionRSA-4                                    806323        764824        -5.15%
BenchmarkJWTSessionRSABearer-4                              789780        763499        -3.33%
BenchmarkJWTSessionRSAWithRawSourceOnWithClientID-4         703032        709992        +0.99%
BenchmarkJWTSessionRSAWithRawSource-4                       663589        667970        +0.66%
BenchmarkJWTSessionRSAWithJWK-4                             672989        670970        -0.30%
BenchmarkJWTSessionRSAWithEncodedJWK-4                      665829        682345        +2.48%
BenchmarkProcessRequestLiveQuotaLimit-4                     590197        552551        -6.38%
BenchmarkProcessRequestOffThreadQuotaLimit-4                481112        464043        -3.55%
BenchmarkProcessRequestLiveRedisRollingLimiter-4            835427        811060        -2.92%
BenchmarkProcessRequestOffThreadRedisRollingLimiter-4       597603        589735        -1.32%
BenchmarkStripAuth_stripFromHeaders-4                       7542          7564          +0.29%
BenchmarkStripAuth_stripFromParams-4                        35052         35823         +2.20%
BenchmarkTransformNonAscii-4                                14642         14700         +0.40%
BenchmarkTransformJSONMarshal-4                             15125         15487         +2.39%
BenchmarkRewriter-4                                         284003        292416        +2.96%
BenchmarkValidateJSONSchema-4                               1711539       1747710       +2.11%
BenchmarkVersioning-4                                       1739161       1887525       +8.53%
BenchmarkVirtualEndpoint-4                                  355171        371716        +4.66%
BenchmarkApplyPolicies-4                                    23440         23324         -0.49%
BenchmarkResponseHeaderInjection-4                          2170868       2199569       +1.32%
BenchmarkCopyRequestResponse-4                              15648         15370         -1.78%
BenchmarkRealIP_RemoteAddr-4                                136           137           +0.74%
BenchmarkRealIP_ForwardedFor-4                              127           127           +0.00%
BenchmarkRealIP_RealIP-4                                    95.7          96.2          +0.52%
BenchmarkRealIP_Context-4                                   12.6          12.7          +0.79%

benchmark                                                   old allocs     new allocs     delta
BenchmarkURLReplacer-4                                      44             44             +0.00%
BenchmarkTagHeaders-4                                       21             15             -28.57%
BenchmarkDefaultVersion-4                                   1795           1780           -0.84%
BenchmarkApiReload-4                                        23908          23908          +0.00%
BenchmarkMultiSession_BA_Standard_OK-4                      467            455            -2.57%
BenchmarkBearerTokenAuthKeySession-4                        388            386            -0.52%
BenchmarkMultiAuthBackwardsCompatibleSession-4              393            390            -0.76%
BenchmarkBasicAuth-4                                        26009          25981          -0.11%
BenchmarkContextVarsMiddleware-4                            2007           2007           +0.00%
BenchmarkMiddlewareContextVars_ProcessRequest_cookies-4     28             28             +0.00%
BenchmarkHMACAuthSessionPass-4                              392            390            -0.51%
BenchmarkHMACAuthSessionPassWithHeaderField-4               417            414            -0.72%
BenchmarkIPBlacklistMiddleware-4                            148            148            +0.00%
BenchmarkIPMiddlewarePass-4                                 94             94             +0.00%
BenchmarkJWTSessionHMAC-4                                   710            706            -0.56%
BenchmarkJWTSessionRSA-4                                    785            781            -0.51%
BenchmarkJWTSessionRSABearer-4                              787            783            -0.51%
BenchmarkJWTSessionRSAWithRawSourceOnWithClientID-4         809            806            -0.37%
BenchmarkJWTSessionRSAWithRawSource-4                       793            790            -0.38%
BenchmarkJWTSessionRSAWithJWK-4                             797            794            -0.38%
BenchmarkJWTSessionRSAWithEncodedJWK-4                      801            798            -0.37%
BenchmarkProcessRequestLiveQuotaLimit-4                     574            571            -0.52%
BenchmarkProcessRequestOffThreadQuotaLimit-4                536            533            -0.56%
BenchmarkProcessRequestLiveRedisRollingLimiter-4            2269           2290           +0.93%
BenchmarkProcessRequestOffThreadRedisRollingLimiter-4       2558           2563           +0.20%
BenchmarkStripAuth_stripFromHeaders-4                       66             66             +0.00%
BenchmarkStripAuth_stripFromParams-4                        220            220            +0.00%
BenchmarkTransformNonAscii-4                                98             98             +0.00%
BenchmarkTransformJSONMarshal-4                             101            101            +0.00%
BenchmarkRewriter-4                                         858            858            +0.00%
BenchmarkValidateJSONSchema-4                               2601           2585           -0.62%
BenchmarkVersioning-4                                       2736           2717           -0.69%
BenchmarkVirtualEndpoint-4                                  383            416            +8.62%
BenchmarkApplyPolicies-4                                    102            102            +0.00%
BenchmarkResponseHeaderInjection-4                          2804           2805           +0.04%
BenchmarkCopyRequestResponse-4                              76             76             +0.00%
BenchmarkRealIP_RemoteAddr-4                                1              1              +0.00%
BenchmarkRealIP_ForwardedFor-4                              1              1              +0.00%
BenchmarkRealIP_RealIP-4                                    1              1              +0.00%
BenchmarkRealIP_Context-4                                   0              0              +0.00%

benchmark                                                   old bytes     new bytes     delta
BenchmarkURLReplacer-4                                      849           849           +0.00%
BenchmarkTagHeaders-4                                       480           384           -20.00%
BenchmarkDefaultVersion-4                                   262743        249703        -4.96%
BenchmarkApiReload-4                                        1789661       1789630       -0.00%
BenchmarkMultiSession_BA_Standard_OK-4                      99250         90335         -8.98%
BenchmarkBearerTokenAuthKeySession-4                        85236         79489         -6.74%
BenchmarkMultiAuthBackwardsCompatibleSession-4              86392         80645         -6.65%
BenchmarkBasicAuth-4                                        1354929       1338726       -1.20%
BenchmarkContextVarsMiddleware-4                            308001        308056        +0.02%
BenchmarkMiddlewareContextVars_ProcessRequest_cookies-4     2389          2389          +0.00%
BenchmarkHMACAuthSessionPass-4                              85207         79580         -6.60%
BenchmarkHMACAuthSessionPassWithHeaderField-4               86499         80910         -6.46%
BenchmarkIPBlacklistMiddleware-4                            32246         32249         +0.01%
BenchmarkIPMiddlewarePass-4                                 26697         26698         +0.00%
BenchmarkJWTSessionHMAC-4                                   115648        107141        -7.36%
BenchmarkJWTSessionRSA-4                                    154777        136651        -11.71%
BenchmarkJWTSessionRSABearer-4                              156045        137849        -11.66%
BenchmarkJWTSessionRSAWithRawSourceOnWithClientID-4         137253        128294        -6.53%
BenchmarkJWTSessionRSAWithRawSource-4                       129327        122547        -5.24%
BenchmarkJWTSessionRSAWithJWK-4                             129373        122596        -5.24%
BenchmarkJWTSessionRSAWithEncodedJWK-4                      129550        122763        -5.24%
BenchmarkProcessRequestLiveQuotaLimit-4                     92447         88599         -4.16%
BenchmarkProcessRequestOffThreadQuotaLimit-4                90763         86810         -4.36%
BenchmarkProcessRequestLiveRedisRollingLimiter-4            160444        157239        -2.00%
BenchmarkProcessRequestOffThreadRedisRollingLimiter-4       171922        168274        -2.12%
BenchmarkStripAuth_stripFromHeaders-4                       2800          2800          +0.00%
BenchmarkStripAuth_stripFromParams-4                        14161         14161         +0.00%
BenchmarkTransformNonAscii-4                                11058         11058         +0.00%
BenchmarkTransformJSONMarshal-4                             11506         11506         +0.00%
BenchmarkRewriter-4                                         717567        717567        +0.00%
BenchmarkValidateJSONSchema-4                               290195        289923        -0.09%
BenchmarkVersioning-4                                       246852        237072        -3.96%
BenchmarkVirtualEndpoint-4                                  70767         72269         +2.12%
BenchmarkApplyPolicies-4                                    12581         12582         +0.01%
BenchmarkResponseHeaderInjection-4                          645475        645555        +0.01%
BenchmarkCopyRequestResponse-4                              35584         35584         +0.00%
BenchmarkRealIP_RemoteAddr-4                                16            16            +0.00%
BenchmarkRealIP_ForwardedFor-4                              16            16            +0.00%
BenchmarkRealIP_RealIP-4                                    16            16            +0.00%
BenchmarkRealIP_Context-4                                   0             0             +0.00%

Using benchcmp on this branch(new) compared with master(old) - i've started expanding out the replacements to other places in the code where I can.

@joshblakeley
Copy link
Member Author

joshblakeley commented May 8, 2018 •
edited
Loading

Ok @dencoded thanks for the pointers - i'll do just that!

@matiasinsaurralde
Copy link
Contributor

Looks good!

This was referenced May 9, 2018
Merged
Closed
@buger
Copy link
Member

buger commented May 9, 2018

@joshblakeley can you also write func (s *SessionState) Hash() benchmark as suggested by @dencoded. It does not need to be included into this PR, but I want to see numbers before and after the change.

@joshblakeley
Copy link
Member Author

joshblakeley commented May 9, 2018 •
edited
Loading

@buger i did earlier today see #1687

Numbers on my machine went from ~4500ns/op to ~3300 ns/op

@buger
Copy link
Member

buger commented May 9, 2018

👍

@buger buger merged commit 1c01ddc into master May 9, 2018
@buger buger deleted the fmtchange branch May 9, 2018 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@joshblakeley joshblakeley

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@joshblakeley @buger @dencoded @matiasinsaurralde