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

[DI] Improve internal caching algorithm resource overhead #4864

Conversation

watson
Copy link
Collaborator

@watson watson commented Nov 6, 2024

Use ttl-set instead of lru-cache for keeping track of when the Dynamic Instrumentation backend should be updated with new probe acknowledgements.

The ttl-set package acts like a JavaScript Set, but with a TTL on entries and is extremely fast and with low overhead.

Bug fix

For Node.js 18, this also fixes a bug/race condition related to lru-cache that would occur under heavy load:

TypeError: Cannot read properties of undefined (reading 'Symbol(kResourceStore)')
    at AsyncLocalStorage._propagate (node:async_hooks:312:34)
    at AsyncHook.init (node:async_hooks:271:22)
    at emitInitNative (node:internal/async_hooks:200:43)
    at emitInitScript (node:internal/async_hooks:503:3)
    at initAsyncResource (node:internal/timers:164:5)
    at new Timeout (node:internal/timers:198:5)
    at setTimeout (node:timers:165:19)
    at getNow (/Users/thomas.watson/go/src/github.com/DataDog/node_modules/dd-trace/node_modules/lru-cache/index.js:355:19)
    at LRUCache.isStale (/Users/thomas.watson/go/src/github.com/DataDog/node_modules/dd-trace/node_modules/lru-cache/index.js:383:23)
    at LRUCache.has (/Users/thomas.watson/go/src/github.com/DataDog/node_modules/dd-trace/node_modules/lru-cache/index.js:795:17)

@watson watson requested review from a team as code owners November 6, 2024 10:47
Copy link
Collaborator Author

watson commented Nov 6, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link

github-actions bot commented Nov 6, 2024

Overall package size

Self size: 8.49 MB
Deduped: 94.85 MB
No deduping: 95.36 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.4.0 | 29.44 MB | 29.44 MB | | @datadog/native-appsec | 8.4.0 | 19.25 MB | 19.26 MB | | @datadog/native-iast-taint-tracking | 3.2.0 | 13.9 MB | 13.91 MB | | @datadog/pprof | 5.4.1 | 9.76 MB | 10.13 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.6.1 | 2.59 MB | 2.73 MB | | @opentelemetry/core | 1.14.0 | 872.87 kB | 1.47 MB | | @datadog/native-metrics | 3.1.0 | 1.06 MB | 1.46 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.11.2 | 112.74 kB | 826.22 kB | | source-map | 0.7.4 | 226 kB | 226 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | lru-cache | 7.18.3 | 133.92 kB | 133.92 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.0 | 109.9 kB | 109.9 kB | | semver | 7.6.3 | 95.82 kB | 95.82 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.1 | 51.46 kB | 51.46 kB | | shell-quote | 1.8.1 | 44.96 kB | 44.96 kB | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | rfdc | 1.3.1 | 25.21 kB | 25.21 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | dc-polyfill | 0.1.4 | 23.1 kB | 23.1 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | ttl-set | 1.0.0 | 4.61 kB | 9.69 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | module-details-from-path | 1.0.3 | 4.47 kB | 4.47 kB |

🤖 This report was automatically generated by heaviest-objects-in-the-universe

@watson watson force-pushed the watson/11-05-_di_improve_internal_caching_algorithm_resource_overhead branch from 222b2a3 to 10f7df8 Compare November 6, 2024 10:49
@watson watson self-assigned this Nov 6, 2024
@pr-commenter
Copy link

pr-commenter bot commented Nov 6, 2024

Benchmarks

Benchmark execution time: 2025-01-21 07:47:46

Comparing candidate commit e8a5bc2 in PR branch watson/11-05-_di_improve_internal_caching_algorithm_resource_overhead with baseline commit 0d49ecf in branch master.

Found 1 performance improvements and 2 performance regressions! Performance is the same for 903 metrics, 27 unstable metrics.

scenario:debugger-line-probe-without-snapshot-18

  • 🟩 cpu_user_time [-55.426ms; -38.204ms] or [-7.788%; -5.368%]

scenario:startup-with-tracer-20

  • 🟥 cpu_user_time [+13.839ms; +22.410ms] or [+5.907%; +9.565%]
  • 🟥 execution_time [+14.581ms; +19.766ms] or [+5.206%; +7.057%]

@watson watson force-pushed the watson/11-05-_di_improve_internal_caching_algorithm_resource_overhead branch from 10f7df8 to e8a5bc2 Compare January 21, 2025 07:36
@watson watson merged commit 51e6350 into master Jan 21, 2025
237 checks passed
@watson watson deleted the watson/11-05-_di_improve_internal_caching_algorithm_resource_overhead branch January 21, 2025 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants