Libdatadog String interning by r1viollet · Pull Request #485 · DataDog/ddprof

r1viollet · 2026-01-29T13:43:43Z

What does this PR do?

Switches the pprof pipeline to libdatadog's ProfilesDictionary so strings, functions, and mappings are interned once instead of copied per sample.

Symbol stores a FunctionId2 handle; MapInfoTable is now vector<MappingId2> — no owned strings in either.
All symbol/mapping lookups intern into the dictionary on first insert and return opaque handles. Subsequent lookups are cache hits at the ddprof level.
Labels are interned once (key IDs at profile creation, values as CharSlice at sample time).
Aggregation goes through Profile_add2 on a Profile_with_dictionary-backed profile — no re-interning at sample time.
ProfilesDictionary held by SymbolHdr (declared first, destroyed last, outlives all handles).
Mapping cache (MapInfoLookup) detects remaps at the same start address by reading identity back from the dictionary.

Motivation

Copying stack symbol strings on every sample was the wrong design. Interned IDs let pprof aggregation be pointer-sized work.

How to test the change?

ddprof_pprof-ut includes a new aggregate → reset → aggregate cycle exercising ID stability across pprof_reset.
mapinfo_lookup-ut covers cache hits, remap detection, build-id mismatch, pid isolation, and erase behaviour.
New pprof_aggregate-bench micro-benchmark: on aarch64 Release, mean per-sample aggregation drops 877 ns → 333 ns (~2.6×).
Existing ddprof_exporter-ut already destroys SymbolHdr before serialization — covers dictionary-outliving-samples under ASan.

pr-commenter · 2026-02-03T09:03:13Z

Benchmark results for collatz

Parameters

	Baseline	Candidate
config	baseline	candidate
profiler-version	ddprof 0.23.0+77955e2e.103721732	ddprof 0.23.0+0e677aed.103753196

Summary

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

See unchanged results

scenario	Δ mean execution_time
scenario:ddprof -S bench-collatz --preset cpu_only collatz_runner.sh	same

pr-commenter · 2026-02-03T09:03:15Z

Benchmark results for BadBoggleSolver_run

Parameters

	Baseline	Candidate
config	baseline	candidate
profiler-version	ddprof 0.23.0+77955e2e.103721732	ddprof 0.23.0+0e677aed.103753196

Summary

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

See unchanged results

scenario	Δ mean execution_time
scenario:ddprof -S bench-bad-boggle-solver BadBoggleSolver_run work 1000	same

r1viollet · 2026-02-03T13:54:04Z

Deploying to relenv to see what this yields

…orage

…rning

Mirrors TEST(DDProfPProf, aggregate) but drives the aggregation in a tight loop via google benchmark so we can measure ns/sample on the interned path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Exercises pprof_reset between two pprof_aggregate_interned_sample calls to confirm that the interned StringId2/FunctionId2/MappingId2 handles stored in the SymbolHdr-owned ProfilesDictionary remain valid across a profile reset and get serialized correctly on the second cycle. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The libdatadog ProfilesDictionaryHandle is already a pointer type (ProfilesDictionary*), and the C API takes it by pointer-to-handle so it can out-write / zero the slot. The previous code heap-allocated that slot and wrapped it in a unique_ptr<Handle, Deleter> — effectively a smart pointer to a heap-allocated ProfilesDictionary** with no benefit. Introduce a small move-only RAII class that holds the handle inline. No behavior change; callers that need the pointer-to-handle the C API wants still get it via .get(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

nsavoire · 2026-04-27T11:15:23Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a6eb45d0f1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-27T11:24:27Z

+  ddog_prof_Status status = ddog_prof_ProfilesDictionary_insert_str(
+      &string_id, dict, to_CharSlice(str), DDOG_PROF_UTF8_OPTION_ASSUME);


Validate strings before interning

When a profiled process has a mapped filename, debug source path, or symbol text containing non-UTF-8 bytes, this helper now passes that untrusted data to libdatadog with DDOG_PROF_UTF8_OPTION_ASSUME; libdatadog's ASSUME mode is documented/implemented as undefined behavior for invalid UTF-8 (it uses unchecked conversion), whereas the old Profile_add path validated strings. Use validate or lossy conversion here so arbitrary Linux path bytes cannot make the profiler invoke UB.

Useful? React with 👍 / 👎.

nsavoire · 2026-04-27T12:24:38Z

  Offset_t _offset{0};
  std::string _sopath;
  BuildIdStr _build_id;
+  ddog_prof_Mapping2 *_mapping_id{nullptr};


Suggested change

ddog_prof_Mapping2 *_mapping_id{nullptr};

ddog_prof_MappingId2 _mapping_id{nullptr};

nsavoire · 2026-04-27T12:55:06Z

+  explicit operator bool() const { return _handle != nullptr; }
+
+  // The C API expects a pointer-to-handle (so it can mutate the slot).
+  [[nodiscard]] const ddog_prof_ProfilesDictionaryHandle *get() const {


Not sure if this is necessary, the only used C API that requires a pointer-to-handle is ddog_prof_Profile_with_dictionary and it should not mutate the slot.

I think this comment was about the get, I did not really adjust this, tell me if you feel strongly. I might have missed why this was important.

nsavoire · 2026-04-27T13:20:56Z

+Symbol make_symbol(std::string symname, const std::string &demangled_name,
+                   uint32_t lineno, const std::string &srcpath,
+                   const ddog_prof_ProfilesDictionary *dict) {
+  [[maybe_unused]] const std::string ignored_symname = std::move(symname);


symname is never used

Move the storage to the libdatadog objects

r1viollet · 2026-04-29T12:53:24Z

@codex review

chatgpt-codex-connector · 2026-04-29T12:53:31Z

To use Codex here, create a Codex account and connect to github.

r1viollet · 2026-04-29T13:00:49Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb1e5f2e65

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-29T13:07:07Z

+    DDRES_CHECK_FWD(pprof_create_profile(
+        ctx.worker_ctx.pprof[0], ctx,
+        ctx.worker_ctx.us->symbol_hdr._profiles_dictionary.get()));


Avoid sharing the intern dictionary across export buffers

When asynchronous export is used, both pprof buffers are created with this same SymbolHdr dictionary (and pprof[1] gets the same pointer just below). ddprof_worker_cycle then swaps buffers and starts ddprof_worker_export_thread to serialize the old profile while new samples continue aggregating into the other profile; those aggregations call pprof_aggregate_interned_sample and intern strings/functions/mappings into this shared dictionary. That defeats the existing double-buffering and leaves the exporter reading the dictionary concurrently with writes, which can corrupt serialized profiles or race in libdatadog. Use a per-buffer/export-snapshot dictionary or guard all serialization and interning with the same lock.

Useful? React with 👍 / 👎.

The key libdatadog header says ProfilesDictionary contains data common to multiple profiles, “whether that’s multiple profiles simultaneously or multiple profiles through time”, and that the current implementation is thread-safe. That matches our double-buffer setup: two Profiles sharing one long-lived SymbolHdr dictionary.

I think we are fine, though codex does not have access to the libdatadog APIs.

r1viollet · 2026-04-29T13:29:36Z

To answer a question, mappings are never cleared afaik, so we still partially rely on the hard worker reset.

r1viollet changed the title ~~R1viollet/string interning~~ Libdatadog String interning Feb 3, 2026

r1viollet added 9 commits March 5, 2026 14:35

String interning WIP

3524e98

String interning - use libdatadog's dictionary to intern strings

06c359e

String interning - intern labels

535defa

String interning - clean up double interning on labels

0d883a9

String interning - remove usage of strings in favour of libdatadog st…

0b326ad

…orage

String interning - plug libdatadog dict in the symbolizer APIs

0d86655

String interning - Simplify the symbol object

1326386

Linter fix - const correctness fix

bad9de2

Merge remote-tracking branch 'origin/main' into r1viollet/string-inte…

0e677ae

…rning

r1viollet force-pushed the r1viollet/string-interning branch from 341431b to 0e677ae Compare March 20, 2026 15:33

r1viollet and others added 3 commits April 24, 2026 16:31

Merge remote-tracking branch 'origin/main' into r1viollet/string-inte…

e900fd1

…rning

test: add pprof_aggregate micro-benchmark

8c2c8f8

Mirrors TEST(DDProfPProf, aggregate) but drives the aggregation in a tight loop via google benchmark so we can measure ns/sample on the interned path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

r1viollet marked this pull request as ready for review April 24, 2026 15:40

r1viollet requested a review from nsavoire as a code owner April 24, 2026 15:40

r1viollet force-pushed the r1viollet/string-interning branch from 55f0990 to a6eb45d Compare April 24, 2026 15:51

r1viollet assigned nsavoire Apr 27, 2026

chatgpt-codex-connector Bot reviewed Apr 27, 2026

View reviewed changes

nsavoire reviewed Apr 27, 2026

View reviewed changes

Comment thread include/mapinfo_table.hpp Outdated

nsavoire reviewed Apr 27, 2026

View reviewed changes

r1viollet marked this pull request as draft April 29, 2026 12:44

libdatadog interning: refactor the storage of mappings

cb1e5f2

Move the storage to the libdatadog objects

Minor cleanup of comments

3a607fa

chatgpt-codex-connector Bot reviewed Apr 29, 2026

View reviewed changes

Minor cleanup in types

97b83c2

r1viollet marked this pull request as ready for review April 29, 2026 13:27

		ddog_prof_Status status = ddog_prof_ProfilesDictionary_insert_str(
		&string_id, dict, to_CharSlice(str), DDOG_PROF_UTF8_OPTION_ASSUME);

	ddog_prof_Mapping2 *_mapping_id{nullptr};
	ddog_prof_MappingId2 _mapping_id{nullptr};

Conversation

r1viollet commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

How to test the change?

Uh oh!

pr-commenter Bot commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmark results for collatz

Parameters

Summary

Uh oh!

pr-commenter Bot commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmark results for BadBoggleSolver_run

Parameters

Summary

Uh oh!

r1viollet commented Feb 3, 2026

Uh oh!

nsavoire commented Apr 27, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

nsavoire Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nsavoire Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

r1viollet Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

nsavoire Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

r1viollet commented Apr 29, 2026

Uh oh!

chatgpt-codex-connector Bot commented Apr 29, 2026

Uh oh!

r1viollet commented Apr 29, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

r1viollet Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

r1viollet commented Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

r1viollet commented Jan 29, 2026 •

edited

Loading

pr-commenter Bot commented Feb 3, 2026 •

edited

Loading

pr-commenter Bot commented Feb 3, 2026 •

edited

Loading