Skip to content

Purge unsupported platforms - #818

Merged
zhengyu123 merged 4 commits into
mainfrom
zgu/purge-unsupported
Sep 24, 2026
Merged

zhengyu123 merged 4 commits into
mainfrom
zgu/purge-unsupported

Conversation

@zhengyu123

@zhengyu123 zhengyu123 commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?:
Removes all code paths for unsupported 32-bit targets — x86 (__i386__) and ARM/Thumb (__arm__/__thumb__) — across the native profiler core, the Gradle build-logic (Kotlin), and the Java native-library loader:

  • C++: arch.h, dwarf.h, tsc.h, trap.cpp, os_linux.cpp, otel_process_ctx.cpp, symbols_linux.cpp, safeAccess.cpp, hotspot/vmStructs.cpp — dropped the #if/#elif branches, constants (BREAKPOINT_THUMB, R_386_*, R_ARM_*), and the now-unreachable __LP64__/ELF32 fallback paths.
  • Build-logic: Architecture.kt and PlatformUtils.kt — removed the X86/ARM enum values and their detection/dispatch branches.
  • Java: Arch.java — removed the x86/arm enum constants used for native-lib resource resolution.
  • Updated two stale test comments in returnAddressAttribution_ut.cpp that still referenced i386 as a live target.

Motivation:
None of these 32-bit targets are built, tested, or shipped by this project. The dead preprocessor branches and enum variants were pure maintenance burden — untested, easy to get subtly wrong on future edits (e.g. the arm/thumb code was flagged in its own comments as "untested on real hardware"), and misleading in docs/comments that still cited them as real targets.

Additional Notes:
Deletion-only change; no behavior difference for the remaining supported targets (x86_64, aarch64) or for PPC64LE/RISC-V/LoongArch, which were untouched. Verified via repo-wide grep that no references to __i386__, __arm__, __thumb__, BREAKPOINT_THUMB, R_386/R_ARM_*, or the removed Architecture/Arch enum values remain anywhere in source, build config, or CI.

How to test the change?:
Existing unit/integration test suite on x86_64 and aarch64 (the only targets this repo builds/tests) — no new coverage is needed since this only removes code for architectures that were never exercised by CI. Confirmed arch.h/tsc.h compile standalone, build-logic/conventions compiles (gradlew compileKotlin), and Arch.java compiles (javac).

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a security review (run the dd:platform-security-review
    skill, or file a request via the PSEC review form).
    bewaire also runs automatically on every PR.
  • This PR doesn't touch any of that.
  • JIRA: PROF-16049

Unsure? Have a question? Request a review!

@datadog-prod-us1-6

This comment has been minimized.

@dd-octo-sts

dd-octo-sts Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

CI Test Results

Run: #36008462458 | Commit: 59862a8 | Duration: 42m 12s (longest job)

✅ All 32 test jobs passed

Status Overview

JDK glibc-aarch64/debug glibc-amd64/debug musl-aarch64/debug musl-amd64/debug
8 - ✅ - -
8-ibm - ✅ - -
8-j9 ✅ ✅ - -
8-librca - - ✅ ✅
8-orcl - ✅ - -
11 - ✅ - -
11-j9 ✅ ✅ - -
11-librca - - ✅ ✅
17 ✅ ✅ - -
17-graal ✅ ✅ - -
17-j9 ✅ ✅ - -
17-librca - - ✅ ✅
21 ✅ ✅ - -
21-graal ✅ ✅ - -
21-librca - - ✅ ✅
25 ✅ ✅ - -
25-graal ✅ ✅ - -
25-librca - - ✅ ✅

Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled

Summary: Total: 32 | Passed: 32 | Failed: 0


Updated: 2026-09-24 14:35:45 UTC

@zhengyu123
zhengyu123 marked this pull request as ready for review September 23, 2026 18:48
@zhengyu123
zhengyu123 requested a review from a team as a code owner September 23, 2026 18:48

@datadog-prod-us1-6 datadog-prod-us1-6 Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bits Code Review: PASS

More details

Architecture detection now rejects x86 and 32-bit ARM. The x86_64 and aarch64 paths keep the same enum values, library names, and sanitizer suffixes.

Was this helpful? React 👍 or 👎

Open Bits AI session

🤖 Bits Code Review · Commit 5c8c827 · @DataDog review to ask questions

@dd-octo-sts

dd-octo-sts Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

✅ All 40 integration tests passed

📊 Dashboard · 👷 Pipeline · 📦 cafd3859

@kaahos kaahos left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The agent tells me some copyrights updates are missing in symbols_linux.cpp, trap.cpp, tsc.h, otel_process_ctx.cpp, Arch.java. Other than that, this looks good to me, thanks!

Comment thread ddprof-lib/src/main/cpp/arch.h Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4ec08cfe33

ℹ️ 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".

Comment thread ddprof-lib/src/main/cpp/arch.h Outdated
@zhengyu123
zhengyu123 merged commit 2bed643 into main Sep 24, 2026
110 checks passed
@zhengyu123
zhengyu123 deleted the zgu/purge-unsupported branch September 24, 2026 14:38
@github-actions github-actions Bot added this to the 1.51.0 milestone Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants