Skip to content

Commit 5593911

Browse files
perf(profiling): remove more unnecessary sorting (#4954)
## Description This change removes more unnecessary sorting operations that were added to ensure that the data was reproducible. This was only beneficial for tests, and as such only constituted an extra burden on CPU that just added overhead to the profiler. ## Checklist - [ ] Followed the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) when writing a release note. - [ ] Add additional sections for `feat` and `fix` pull requests. - [ ] [Library documentation](https://github.com/DataDog/dd-trace-py/tree/1.x/docs) and/or [Datadog's documentation site](https://github.com/DataDog/documentation/) is updated. Link to doc PR in description. ## Reviewer Checklist - [ ] Title is accurate. - [ ] Description motivates each change. - [ ] No unnecessary changes were introduced in this PR. - [ ] Avoid breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [ ] Tests provided or description of manual testing performed is included in the code or PR. - [ ] Release note has been added and follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines), or else `changelog/no-changelog` label added. - [ ] All relevant GitHub issues are correctly linked. - [ ] Change contains telemetry where appropriate (logs, metrics, etc.). - [ ] Telemetry is meaningful, actionable and does not have the potential to leak sensitive data. Co-authored-by: Brett Langdon <[email protected]>
1 parent a41e8b4 commit 5593911

File tree

3 files changed

+13
-8
lines changed

3 files changed

+13
-8
lines changed

ddtrace/profiling/exporter/pprof.pyx

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ class _PprofConverter(object):
463463
"name": lib.name,
464464
"kind": "library",
465465
"version": lib.version,
466-
"paths": sorted(lib_and_filename[1] for lib_and_filename in libs_and_filenames),
466+
"paths": [lib_and_filename[1] for lib_and_filename in libs_and_filenames],
467467
}
468468
)
469469
for lib, libs_and_filenames in groupby(
@@ -496,7 +496,7 @@ class _PprofConverter(object):
496496
value=[values.get(sample_type_name, 0) for sample_type_name, unit in sample_types],
497497
label=[pprof_pb2.Label(key=self._str(key), str=self._str(s)) for key, s in labels],
498498
)
499-
for (locations, labels), values in sorted(six.iteritems(self._location_values), key=_ITEMGETTER_ZERO)
499+
for (locations, labels), values in six.iteritems(self._location_values)
500500
]
501501

502502
period_type = pprof_pb2.ValueType(type=self._str("time"), unit=self._str("nanoseconds"))
@@ -512,9 +512,8 @@ class _PprofConverter(object):
512512
filename=self._str(program_name),
513513
),
514514
],
515-
# Sort location and function by id so the output is reproducible
516-
location=sorted(self._locations.values(), key=_ATTRGETTER_ID),
517-
function=sorted(self._functions.values(), key=_ATTRGETTER_ID),
515+
location=self._locations.values(),
516+
function=self._functions.values(),
518517
string_table=list(self._string_table),
519518
time_nanos=start_time_ns,
520519
duration_nanos=duration_ns,
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
other:
3+
- |
4+
profiler: CPU overhead reduction.

tests/profiling/exporter/test_pprof.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,7 @@ def test_pprof_exporter_libs(gan):
758758
]
759759
}
760760

761-
exports, libs = exp.export(TEST_EVENTS, 1, 7)
761+
_, libs = exp.export(TEST_EVENTS, 1, 7)
762762

763763
# Version does not match between pip and __version__ for ddtrace; ignore
764764
for lib in libs:
@@ -772,8 +772,8 @@ def test_pprof_exporter_libs(gan):
772772
del lib["paths"]
773773

774774
expected_libs = [
775-
{"name": "ddtrace", "kind": "library", "paths": [memalloc.__file__, __file__]},
776-
{"name": "six", "kind": "library", "version": six.__version__, "paths": [six.__file__]},
775+
{"name": "ddtrace", "kind": "library", "paths": {__file__, memalloc.__file__}},
776+
{"name": "six", "kind": "library", "version": six.__version__, "paths": {six.__file__}},
777777
{"kind": "standard library", "name": "stdlib", "version": platform.python_version()},
778778
{
779779
"kind": "library",
@@ -794,6 +794,8 @@ def test_pprof_exporter_libs(gan):
794794
# - we end up with an empty expected_libs
795795
# This is equivalent to checking that the two lists are equal.
796796
for _ in libs:
797+
if "paths" in _:
798+
_["paths"] = set(_["paths"])
797799
expected_libs.remove(_)
798800

799801
assert not expected_libs

0 commit comments

Comments
 (0)