Skip to content

Revert lazy profile-tree changes to mitigate CpuProfile::Delete crash#329

Merged
szegedi merged 3 commits into
mainfrom
revert-lazy-profile-trees
May 19, 2026
Merged

Revert lazy profile-tree changes to mitigate CpuProfile::Delete crash#329
szegedi merged 3 commits into
mainfrom
revert-lazy-profile-trees

Conversation

@szegedi
Copy link
Copy Markdown

@szegedi szegedi commented May 19, 2026

Summary

Reverts the two commits that switched profile collection to lazy/callback-based paths, restoring the eager translation flow for both time and heap profilers. See PROF-14622 for the crash analysis that motivates this — two customer reports (dd-trace 5.98.0 / Node 20.20.2 and dd-trace 5.101.0 / Node 22.20.0, both on @datadog/pprof 5.14.1) abort inside glibc's free() heap-consistency check while V8 is destroying CpuProfile state at the end of WallProfiler::StopAndCollectImpl. The lazy view widens the window in which a new V8 profile is sampling while the old one is being deleted (the JS callback runs in between), and aborts are observed across two V8 majors.

  • Revert "use stop and collect on time profiler (#305)" — restores Stop / StopImpl and the eager TranslateTimeProfile path; stopAndCollect / profileV2 remain available but no longer the only entry.
  • Revert "Switch heap profiling to use lazy allocation profile method by default (#281)" — restores getAllocationProfile as the default for heap.
  • Add explicit return types to eager time-profiler stop() / profile() — small TS 6 compat fix: the TS 6 bump (build(deps-dev): bump typescript from 5.9.3 to 6.0.3 #315) postdates the reverted commits, so the restored functions now trip TS2883 under composite: true. One Profile import and two return annotations, no runtime change.

The lazy code (BuildTimeProfileView, mapAllocationProfile, stopV2 / profileV2) stays in place so downstream consumers that want it can still opt in — only the defaults are reverted.

Test plan

  • npm run build — native build clean
  • npm run compile — TS clean under TS 6
  • npm test — 96 passing, 0 failing (mocha suites including memory comparison v1 vs v2 still pass)
  • dd-trace integration: bump pprof to the prerelease cut from this branch and confirm the customer crashes do not reproduce
  • Decide whether the lazy code (stopAndCollect / mapAllocationProfile) should be kept for opt-in use or also removed in a follow-up once PROF-14622 root cause is understood

Jira: PROF-14622

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 19, 2026

Overall package size

Self size: 2.09 MB
Deduped: 2.45 MB
No deduping: 2.45 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | source-map | 0.7.6 | 185.63 kB | 185.63 kB | | pprof-format | 2.2.1 | 163.06 kB | 163.06 kB | | node-gyp-build | 4.8.4 | 13.86 kB | 13.86 kB |

🤖 This report was automatically generated by heaviest-objects-in-the-universe

@szegedi szegedi added the semver-patch Bug or security fixes, mainly label May 19, 2026
Specifically enforcing TS2883 under `composite: true`: any
function whose inferred return type names a non-imported
type must be annotated explicitly.
@szegedi szegedi force-pushed the revert-lazy-profile-trees branch from 6c8bcb8 to 49f97c4 Compare May 19, 2026 07:43
@szegedi szegedi merged commit 232bea8 into main May 19, 2026
68 checks passed
@szegedi szegedi deleted the revert-lazy-profile-trees branch May 19, 2026 12:21
@IlyasShabi IlyasShabi mentioned this pull request May 19, 2026
IlyasShabi pushed a commit that referenced this pull request May 19, 2026
…#329)

* Revert "use stop and collect on time profiler (#305)"

This reverts commit 85f2457.

* Revert "Switch heap profiling to use lazy allocation profile method by default (#281)"

This reverts commit fb3d75d.

* Touchup for TS6 changes introduced after the reverted commits.

Specifically enforcing TS2883 under `composite: true`: any
function whose inferred return type names a non-imported
type must be annotated explicitly.
IlyasShabi added a commit that referenced this pull request May 19, 2026
* use trusted publishing for dev releases (#310)

* use trusted publishing for dev releases

* use release publisher file to release dev tag too (#311)

use release publisher file to release dev tag

* chore(deps): minor: sinon · patch: @types/node, @types/sinon (#317)

Co-authored-by: gh-worker-campaigns-3e9aa4[bot] <244854796+gh-worker-campaigns-3e9aa4[bot]@users.noreply.github.com>

* build(deps-dev): bump typescript from 5.9.3 to 6.0.3 (#315)

* build(deps-dev): bump typescript from 5.9.3 to 6.0.3

Bumps [typescript](https://github.com/microsoft/TypeScript) from 5.9.3 to 6.0.3.
- [Commits](microsoft/TypeScript@v5.9.3...v6.0.3)

---
updated-dependencies:
- dependency-name: typescript
  dependency-version: 6.0.3
  dependency-type: direct:development
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

* fix(ts): explicit types in tsconfig and bump @types/node to 25.6.0

TypeScript 6.0 no longer auto-loads @types/* by default with this
project's tsconfig setup, so Node and Mocha globals (Buffer, process,
__dirname, describe, it, ...) became unresolved. Pin "types" to
["node", "mocha"] so the compiler picks them up explicitly.

Also bumps @types/node from 25.5.2 to 25.6.0, superseding #314.

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Attila Szegedi <attila.szegedi@datadoghq.com>

* chore(deps): node-gyp-build (major → 4.8.4) (#316)

Co-authored-by: gh-worker-campaigns-3e9aa4[bot] <244854796+gh-worker-campaigns-3e9aa4[bot]@users.noreply.github.com>

* build(deps-dev): bump @types/node from 25.6.0 to 25.7.0 (#324)

Bumps [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node) from 25.6.0 to 25.7.0.
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node)

---
updated-dependencies:
- dependency-name: "@types/node"
  dependency-version: 25.7.0
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps-dev): bump eslint-plugin-n from 17.24.0 to 18.0.1 (#320)

Bumps [eslint-plugin-n](https://github.com/eslint-community/eslint-plugin-n) from 17.24.0 to 18.0.1.
- [Release notes](https://github.com/eslint-community/eslint-plugin-n/releases)
- [Changelog](https://github.com/eslint-community/eslint-plugin-n/blob/master/CHANGELOG.md)
- [Commits](eslint-community/eslint-plugin-n@v17.24.0...v18.0.1)

---
updated-dependencies:
- dependency-name: eslint-plugin-n
  dependency-version: 18.0.1
  dependency-type: direct:development
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump node-gyp-build from 3.9.0 to 4.8.4 (#284)

Bumps [node-gyp-build](https://github.com/prebuild/node-gyp-build) from 3.9.0 to 4.8.4.
- [Commits](prebuild/node-gyp-build@v3.9.0...v4.8.4)

---
updated-dependencies:
- dependency-name: node-gyp-build
  dependency-version: 4.8.4
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Attila Szegedi <szegedi@users.noreply.github.com>

* build(deps-dev): bump semver from 7.7.4 to 7.8.0 (#323)

Bumps [semver](https://github.com/npm/node-semver) from 7.7.4 to 7.8.0.
- [Release notes](https://github.com/npm/node-semver/releases)
- [Changelog](https://github.com/npm/node-semver/blob/main/CHANGELOG.md)
- [Commits](npm/node-semver@v7.7.4...v7.8.0)

---
updated-dependencies:
- dependency-name: semver
  dependency-version: 7.8.0
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps-dev): bump sinon from 21.1.2 to 22.0.0 (#325)

Bumps [sinon](https://github.com/sinonjs/sinon) from 21.1.2 to 22.0.0.
- [Release notes](https://github.com/sinonjs/sinon/releases)
- [Changelog](https://github.com/sinonjs/sinon/blob/main/docs/changelog.md)
- [Commits](sinonjs/sinon@v21.1.2...v22.0.0)

---
updated-dependencies:
- dependency-name: sinon
  dependency-version: 22.0.0
  dependency-type: direct:development
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add Node.js v25 to benchmarks (#326)

* support Node.js v26 (#327)

* support Node.js v26

* Revert lazy profile-tree changes to mitigate CpuProfile::Delete crash (#329)

* Revert "use stop and collect on time profiler (#305)"

This reverts commit 85f2457.

* Revert "Switch heap profiling to use lazy allocation profile method by default (#281)"

This reverts commit fb3d75d.

* Touchup for TS6 changes introduced after the reverted commits.

Specifically enforcing TS2883 under `composite: true`: any
function whose inferred return type names a non-imported
type must be annotated explicitly.

* v14.5.2

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: gh-worker-campaigns-3e9aa4[bot] <244854796+gh-worker-campaigns-3e9aa4[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Attila Szegedi <attila.szegedi@datadoghq.com>
Co-authored-by: Attila Szegedi <szegedi@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-patch Bug or security fixes, mainly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants