-
Notifications
You must be signed in to change notification settings - Fork 318
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
lazy load dsm only when needed #5305
base: master
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,53 @@ | |||
'use strict' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The helpers in this file were copied from processor.js
untouched.
Overall package sizeSelf size: 8.77 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.3.0 | 13.77 MB | 13.78 MB | | @datadog/pprof | 5.5.1 | 9.79 MB | 10.17 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.8.0 | 2.6 MB | 2.74 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 | 835.4 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 | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.2 | 53.63 kB | 53.63 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 | | semifies | 1.0.0 | 15.84 kB | 15.84 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 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5305 +/- ##
==========================================
- Coverage 80.94% 80.83% -0.11%
==========================================
Files 488 486 -2
Lines 21844 21658 -186
==========================================
- Hits 17681 17507 -174
+ Misses 4163 4151 -12 ☔ View full report in Codecov by Sentry. |
Datadog ReportBranch report: ✅ 0 Failed, 674 Passed, 0 Skipped, 17m 13.23s Total Time |
BenchmarksBenchmark execution time: 2025-02-25 01:47:58 Comparing candidate commit 1cea28d in PR branch Found 1 performance improvements and 2 performance regressions! Performance is the same for 915 metrics, 15 unstable metrics. scenario:spans-finish-later-22
scenario:startup-with-tracer-22
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM % comment addressed
LazyClass.prototype[method] = function (...args) { | ||
const ActiveClass = classGetter() | ||
const instance = new ActiveClass(...constructorArgs) | ||
|
||
Object.setPrototypeOf(this, instance) | ||
|
||
return this[method](...args) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a comment that explains how this replaces all prototype replaced methods in one go.
Please add a separate TODO (including a time frame for a lintrule similar to how unicorn does it https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/expiring-todo-comments.md) that this is going to be replaced with a system that encapsulates modules and have dedicates plugins for that module or feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually writing the comments made me realize the code can be simplified a bit so I refactored it. I also added the comments and TODO as requested, but I didn't add a time frame as this is not really something we do today, and I felt like this would need to be properly discussed and the linter capability added first.
What does this PR do?
Lazy load DSM only when needed.
Motivation
DSM loads a lot of code and some pretty heavy dependencies which increase startup time when not needed.