perf(profiling): observation map doesn't need HashDos protection#1988
Conversation
Clippy Allow Annotation ReportComparing clippy allow annotations between branches:
Summary by Rule
Annotation Counts by File
Annotation Stats by Crate
About This ReportThis report tracks Clippy allow annotations for specific rules, showing how they've changed in this PR. Decreasing the number of these annotations generally improves code quality. |
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: 58e90ad | Docs | Datadog PR Page | Give us feedback! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1988 +/- ##
=======================================
Coverage 72.69% 72.70%
=======================================
Files 452 452
Lines 74893 74893
=======================================
+ Hits 54446 54449 +3
+ Misses 20447 20444 -3
🚀 New features to boost your workflow:
|
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
Tests failed on this commit d33f94d: What to do next?
|
Retrying merge. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
Tests failed on this commit d49a56c: What to do next?
|
… disk space (#2001) # What does this PR do? Since #1928 we've been intermittently running out of disk space on our GH runners for tests. This PR bumps the size of our ubuntu runners, which includes more disk space and should solve this issue. # Motivation @morrisonlevi was seeing consistent failures on the MQ when running the full test suite on this PR: #1988 # Additional Notes Anything else we should know when reviewing? # How to test the change? Describe here in detail how the change can be validated. Co-authored-by: edmund.kump <edmund.kump@datadoghq.com>
|
/merge |
|
View all feedbacks in Devflow UI.
This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
The expected merge time in
|
What does this PR do?
This changes
AggregatedObservations'sHashMapto useFxHasherfor its hasher instead of the default.Motivation
HashDos protection comes built-in with the default hasher type, and it's unnecessary here. It's more performant in general to use
FxHasher.Additional Notes
My motivation here isn't actually performance, although it's true; I want a deterministic hasher between runs so iteration order is preserved. I am trying to make another version of the
ddog_prof_Profile_serializefunction. I want bit-for-bit fuzz accuracy for property testing it, but with a HashDos protected HashMap, the order of iteration will change from map to map.But even if this other work doesn't land, it's true we're paying for HashDos protection that we don't need, which is why I opened this as its own PR.
How to test the change?
Test as regularly done--this shouldn't change anything.