-
Notifications
You must be signed in to change notification settings - Fork 468
chore(profiling): move echion to dd-trace-py #15136
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
Conversation
|
|
991be14 to
d22f7f9
Compare
Bootstrap import analysisComparison of import times between this PR and base. SummaryThe average import time from this PR is: 252 ± 4 ms. The average import time from base is: 262 ± 7 ms. The import time difference between this PR and base is: -9.7 ± 0.2 ms. Import time breakdownThe following import paths have shrunk:
|
eb048a2 to
08c227e
Compare
Performance SLOsComparing candidate kowalski/echion-to-dd-trace-py (90ff17b) with baseline main (0724124) 📈 Performance Regressions (2 suites)📈 iastaspectsospath - 24/24✅ ospathbasename_aspectTime: ✅ 4.980µs (SLO: <10.000µs 📉 -50.2%) vs baseline: 📈 +15.9% Memory: ✅ 39.872MB (SLO: <41.000MB -2.8%) vs baseline: +4.8% ✅ ospathbasename_noaspectTime: ✅ 1.075µs (SLO: <10.000µs 📉 -89.2%) vs baseline: -0.7% Memory: ✅ 39.872MB (SLO: <41.000MB -2.8%) vs baseline: +4.7% ✅ ospathjoin_aspectTime: ✅ 7.179µs (SLO: <10.000µs 📉 -28.2%) vs baseline: 📈 +15.1% Memory: ✅ 39.852MB (SLO: <41.000MB -2.8%) vs baseline: +4.8% ✅ ospathjoin_noaspectTime: ✅ 2.281µs (SLO: <10.000µs 📉 -77.2%) vs baseline: -0.7% Memory: ✅ 39.793MB (SLO: <41.000MB -2.9%) vs baseline: +4.5% ✅ ospathnormcase_aspectTime: ✅ 3.962µs (SLO: <10.000µs 📉 -60.4%) vs baseline: 📈 +11.1% Memory: ✅ 39.833MB (SLO: <41.000MB -2.8%) vs baseline: +5.0% ✅ ospathnormcase_noaspectTime: ✅ 0.564µs (SLO: <10.000µs 📉 -94.4%) vs baseline: -1.5% Memory: ✅ 39.833MB (SLO: <41.000MB -2.8%) vs baseline: +5.1% ✅ ospathsplit_aspectTime: ✅ 5.676µs (SLO: <10.000µs 📉 -43.2%) vs baseline: 📈 +16.6% Memory: ✅ 39.852MB (SLO: <41.000MB -2.8%) vs baseline: +5.0% ✅ ospathsplit_noaspectTime: ✅ 1.591µs (SLO: <10.000µs 📉 -84.1%) vs baseline: -0.3% Memory: ✅ 39.872MB (SLO: <41.000MB -2.8%) vs baseline: +4.9% ✅ ospathsplitdrive_aspectTime: ✅ 3.990µs (SLO: <10.000µs 📉 -60.1%) vs baseline: +6.8% Memory: ✅ 39.813MB (SLO: <41.000MB -2.9%) vs baseline: +4.7% ✅ ospathsplitdrive_noaspectTime: ✅ 0.693µs (SLO: <10.000µs 📉 -93.1%) vs baseline: -0.8% Memory: ✅ 39.813MB (SLO: <41.000MB -2.9%) vs baseline: +4.4% ✅ ospathsplitext_aspectTime: ✅ 4.601µs (SLO: <10.000µs 📉 -54.0%) vs baseline: -0.5% Memory: ✅ 39.872MB (SLO: <41.000MB -2.8%) vs baseline: +4.9% ✅ ospathsplitext_noaspectTime: ✅ 1.374µs (SLO: <10.000µs 📉 -86.3%) vs baseline: -0.6% Memory: ✅ 39.892MB (SLO: <41.000MB -2.7%) vs baseline: +4.8% 📈 telemetryaddmetric - 30/30✅ 1-count-metric-1-timesTime: ✅ 2.889µs (SLO: <20.000µs 📉 -85.6%) vs baseline: -1.5% Memory: ✅ 34.623MB (SLO: <35.500MB -2.5%) vs baseline: +4.9% ✅ 1-count-metrics-100-timesTime: ✅ 200.287µs (SLO: <220.000µs -9.0%) vs baseline: ~same Memory: ✅ 34.544MB (SLO: <35.500MB -2.7%) vs baseline: +4.8% ✅ 1-distribution-metric-1-timesTime: ✅ 3.312µs (SLO: <20.000µs 📉 -83.4%) vs baseline: +1.4% Memory: ✅ 34.623MB (SLO: <35.500MB -2.5%) vs baseline: +5.2% ✅ 1-distribution-metrics-100-timesTime: ✅ 214.297µs (SLO: <230.000µs -6.8%) vs baseline: -0.3% Memory: ✅ 34.623MB (SLO: <35.500MB -2.5%) vs baseline: +4.7% ✅ 1-gauge-metric-1-timesTime: ✅ 2.566µs (SLO: <20.000µs 📉 -87.2%) vs baseline: 📈 +18.5% Memory: ✅ 34.642MB (SLO: <35.500MB -2.4%) vs baseline: +4.7% ✅ 1-gauge-metrics-100-timesTime: ✅ 136.916µs (SLO: <150.000µs -8.7%) vs baseline: -0.3% Memory: ✅ 34.544MB (SLO: <35.500MB -2.7%) vs baseline: +4.7% ✅ 1-rate-metric-1-timesTime: ✅ 3.465µs (SLO: <20.000µs 📉 -82.7%) vs baseline: 📈 +13.6% Memory: ✅ 34.544MB (SLO: <35.500MB -2.7%) vs baseline: +4.6% ✅ 1-rate-metrics-100-timesTime: ✅ 214.948µs (SLO: <250.000µs 📉 -14.0%) vs baseline: +0.8% Memory: ✅ 34.583MB (SLO: <35.500MB -2.6%) vs baseline: +5.0% ✅ 100-count-metrics-100-timesTime: ✅ 20.313ms (SLO: <22.000ms -7.7%) vs baseline: +0.3% Memory: ✅ 34.603MB (SLO: <35.500MB -2.5%) vs baseline: +4.9% ✅ 100-distribution-metrics-100-timesTime: ✅ 2.247ms (SLO: <2.300ms -2.3%) vs baseline: -0.1% Memory: ✅ 34.564MB (SLO: <35.500MB -2.6%) vs baseline: +4.7% ✅ 100-gauge-metrics-100-timesTime: ✅ 1.411ms (SLO: <1.550ms -9.0%) vs baseline: +0.7% Memory: ✅ 34.662MB (SLO: <35.500MB -2.4%) vs baseline: +5.2% ✅ 100-rate-metrics-100-timesTime: ✅ 2.231ms (SLO: <2.550ms 📉 -12.5%) vs baseline: +1.5% Memory: ✅ 34.642MB (SLO: <35.500MB -2.4%) vs baseline: +5.0% ✅ flush-1-metricTime: ✅ 4.496µs (SLO: <20.000µs 📉 -77.5%) vs baseline: +1.3% Memory: ✅ 34.583MB (SLO: <35.500MB -2.6%) vs baseline: +4.7% ✅ flush-100-metricsTime: ✅ 173.069µs (SLO: <250.000µs 📉 -30.8%) vs baseline: ~same Memory: ✅ 34.524MB (SLO: <35.500MB -2.7%) vs baseline: +4.6% ✅ flush-1000-metricsTime: ✅ 2.126ms (SLO: <2.500ms 📉 -15.0%) vs baseline: +0.2% Memory: ✅ 35.389MB (SLO: <36.500MB -3.0%) vs baseline: +5.1% 🟡 Near SLO Breach (1 suite)🟡 iastpropagation - 8/8✅ no-propagationTime: ✅ 48.631µs (SLO: <60.000µs 📉 -18.9%) vs baseline: -0.4% Memory: ✅ 39.479MB (SLO: <40.500MB -2.5%) vs baseline: +4.8% ✅ propagation_enabledTime: ✅ 166.592µs (SLO: <190.000µs 📉 -12.3%) vs baseline: ~same Memory: ✅ 39.499MB (SLO: <40.000MB 🟡 -1.3%) vs baseline: +5.0% ✅ propagation_enabled_100Time: ✅ 1.866ms (SLO: <2.300ms 📉 -18.9%) vs baseline: +0.8% Memory: ✅ 39.440MB (SLO: <40.000MB 🟡 -1.4%) vs baseline: +4.9% ✅ propagation_enabled_1000Time: ✅ 32.251ms (SLO: <34.550ms -6.7%) vs baseline: +0.5% Memory: ✅ 39.499MB (SLO: <40.000MB 🟡 -1.3%) vs baseline: +4.8%
|
7e12a2c to
4c898d7
Compare
|
/gitlab resync-code |
|
View all feedbacks in Devflow UI.
resync successful |
7327f17 to
cd96233
Compare
|
/gitlab estimate-cost |
|
View all feedbacks in Devflow UI.
Estimated cost is $60.92 for chore(profiling): move echion to dd-trace-py (cumulated CI duration: -494202h1m28.528371712s) |
|
Can you make it explicit from which commit hash this is copied? |
Yes absolutely. What I planned on doing (once I'd get the required approvals) is:
|
|
@KowalskiThomas Is this PR mostly copy-paste from the |
This comment was marked as resolved.
This comment was marked as resolved.
|
Note Since #15311 is not purely copying C++ code, I will wait until it is merged before porting anything relevant from it to this PR, then merge the current PR! Also I still can't merge for the time being as I need an approval from |
emmettbutler
left a comment
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.
Note that in the long run we actually hope to separate the profiling code entirely into its own repo. I can see this change as a step along that path.
ddtrace/internal/datadog/profiling/stack_v2/src/echion/coremodule.cc
Outdated
Show resolved
Hide resolved
ddtrace/internal/datadog/profiling/stack_v2/echion/echion/threads.h
Outdated
Show resolved
Hide resolved
7185793 to
81f47b3
Compare
Specifically commit bbb0c5d2c328765e5548912f9951f20b1baa8b7d
Specifically commit bbb0c5d2c328765e5548912f9951f20b1baa8b7d
81f47b3 to
90ff17b
Compare
What does this PR do?
https://datadoghq.atlassian.net/browse/PROF-12683
This PR moves the source code of Echion from the Echion repository (https://github.com/P403n1x87/echion) to dd-trace-py. We hope that doing this will lead to an improved developer experience in dd-trace-py, as we will effectively:
build/directory / cache, meaning we need to build everything from scratch, which takes time)Note The version of the code that is in this PR is not the latest version of Echion (missing a few PRs). I am still sending the PR for review (so that I can get an approval on the form and general idea) and I will make sure to include/backport any changes merged to Echion before I merge this PR to dd-trace-py.
Note I have copied
.clang-formatover as to minimise the size of the diff and make it easier to port changes from Echion while the PR is in review. I will remove the custom.clang-formatand use the default one from dd-trace-py in a later PR.