Skip to content

Commit

Permalink
filters/auth: release tokeninfo cache mutex earlier (#3241)
Browse files Browse the repository at this point in the history
* filters/auth: reset parallelism for BenchmarkTokeninfoCache

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>

* filters/auth: release tokeninfo cache mutex earlier

Cached value is readonly so release cache mutex early to
copy and adjust cached value outside of critical section.

```
goos: linux
goarch: amd64
pkg: github.com/zalando/skipper/filters/auth
cpu: Intel(R) Core(TM) i5-8350U CPU @ 1.70GHz
                                                  │    HEAD~1    │                 HEAD                 │
                                                  │    sec/op    │    sec/op     vs base                │
TokeninfoCache/tokens=1,cacheSize=1,p=0-8           639.4n ± 21%   271.9n ±  6%  -57.48% (p=0.000 n=10)
TokeninfoCache/tokens=2,cacheSize=2,p=0-8           625.7n ± 24%   420.4n ± 32%  -32.81% (p=0.000 n=10)
TokeninfoCache/tokens=100,cacheSize=100,p=0-8       869.8n ± 17%   463.4n ±  3%  -46.72% (p=0.000 n=10)
TokeninfoCache/tokens=100,cacheSize=100,p=10000-8   670.8n ± 12%   594.6n ±  2%  -11.36% (p=0.000 n=10)
TokeninfoCache/tokens=4,cacheSize=2,p=0-8           2.553m ±  1%   2.554m ±  1%        ~ (p=0.684 n=10)
TokeninfoCache/tokens=100,cacheSize=10,p=0-8        2.581m ±  1%   2.566m ±  0%        ~ (p=0.089 n=10)
geomean                                             10.74µ         7.688µ        -28.44%

                                                  │   HEAD~1   │                HEAD                 │
                                                  │    B/op    │    B/op     vs base                 │
TokeninfoCache/tokens=1,cacheSize=1,p=0-8           344.0 ± 0%   344.0 ± 0%       ~ (p=1.000 n=10) ¹
TokeninfoCache/tokens=2,cacheSize=2,p=0-8           344.0 ± 0%   344.0 ± 0%       ~ (p=1.000 n=10) ¹
TokeninfoCache/tokens=100,cacheSize=100,p=0-8       344.0 ± 0%   344.0 ± 0%       ~ (p=1.000 n=10) ¹
TokeninfoCache/tokens=100,cacheSize=100,p=10000-8   366.0 ± 0%   366.0 ± 1%       ~ (p=0.452 n=10)
TokeninfoCache/tokens=4,cacheSize=2,p=0-8           27.00 ± 4%   27.00 ± 0%       ~ (p=0.474 n=10)
TokeninfoCache/tokens=100,cacheSize=10,p=0-8        28.00 ± 7%   27.00 ± 7%       ~ (p=0.259 n=10)
geomean                                             149.7        148.8       -0.60%
¹ all samples are equal

                                                  │    HEAD~1    │                HEAD                 │
                                                  │  allocs/op   │ allocs/op   vs base                 │
TokeninfoCache/tokens=1,cacheSize=1,p=0-8           3.000 ± 0%     3.000 ± 0%       ~ (p=1.000 n=10) ¹
TokeninfoCache/tokens=2,cacheSize=2,p=0-8           3.000 ± 0%     3.000 ± 0%       ~ (p=1.000 n=10) ¹
TokeninfoCache/tokens=100,cacheSize=100,p=0-8       3.000 ± 0%     3.000 ± 0%       ~ (p=1.000 n=10) ¹
TokeninfoCache/tokens=100,cacheSize=100,p=10000-8   3.000 ± 0%     3.000 ± 0%       ~ (p=1.000 n=10) ¹
TokeninfoCache/tokens=4,cacheSize=2,p=0-8           0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
TokeninfoCache/tokens=100,cacheSize=10,p=0-8        0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                                                        ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean
```

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>

---------

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
  • Loading branch information
AlexanderYastrebov authored Sep 20, 2024
1 parent d161ac5 commit e34f988
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 2 deletions.
10 changes: 8 additions & 2 deletions filters/auth/tokeninfocache.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,19 @@ func (c *tokeninfoCache) cached(token string) map[string]any {
now := c.now()

c.mu.Lock()
defer c.mu.Unlock()

if e, ok := c.cache[token]; ok {
if now.Before(e.expiresAt) {
c.history.MoveToFront(e.href)
cachedInfo := e.info
c.mu.Unlock()

// It might be ok to return cached value
// without adjusting "expires_in" to avoid copy
// if caller never modifies the result and
// when "expires_in" did not change (same second)
// or for small TTL values
info := shallowCopyOf(e.info)
info := shallowCopyOf(cachedInfo)

elapsed := now.Sub(e.cachedAt).Truncate(time.Second).Seconds()
info[expiresInField] = info[expiresInField].(float64) - elapsed
Expand All @@ -81,6 +84,9 @@ func (c *tokeninfoCache) cached(token string) map[string]any {
c.history.Remove(e.href)
}
}

c.mu.Unlock()

return nil
}

Expand Down
2 changes: 2 additions & 0 deletions filters/auth/tokeninfocache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,8 @@ func BenchmarkTokeninfoCache(b *testing.B) {

if bi.parallelism != 0 {
b.SetParallelism(bi.parallelism)
} else {
b.SetParallelism(1)
}

b.ReportAllocs()
Expand Down

0 comments on commit e34f988

Please sign in to comment.