Skip to content

Simplify source pages sidebar handling - #119016

Closed
GuillaumeGomez wants to merge 2 commits into
rust-lang:masterfrom
GuillaumeGomez:simplify-src-sidebar-js
Closed

Simplify source pages sidebar handling#119016
GuillaumeGomez wants to merge 2 commits into
rust-lang:masterfrom
GuillaumeGomez:simplify-src-sidebar-js

Conversation

@GuillaumeGomez

Copy link
Copy Markdown
Member

Since the text isn't really important in itself, instead of updating it manually in the JS, we can let the CSS do it instead.

r? @notriddle

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 16, 2023
@rustbot

rustbot commented Dec 16, 2023

Copy link
Copy Markdown
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

@rustbot rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Dec 16, 2023
#src-sidebar div.files > a.selected {
background-color: var(--src-sidebar-background-selected);
}
#src-sidebar-toggle > button {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I moved the block just under #src-sidebar-toggle rule as it made more sense like this.

@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez
GuillaumeGomez force-pushed the simplify-src-sidebar-js branch from c310228 to ac97299 Compare December 16, 2023 16:41
content: ">";
}
.src-sidebar-expanded #src-sidebar-toggle > button::before {
content: "<";

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.

Didn’t we reduce using generated content for accessibility reasons?

aa73e29

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We absolutely did. The other possibility is to have two buttons. I'll go with this option then.

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.

Could it be switched to <details>?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes but I thought for accessibility reasons we didn't want to do that haha.

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.

Makes sense I guess.

@GuillaumeGomez
GuillaumeGomez force-pushed the simplify-src-sidebar-js branch from ac97299 to e9faeea Compare December 16, 2023 18:05
@GuillaumeGomez

Copy link
Copy Markdown
Member Author

Updated the JS by having two buttons instead of just one.

@bors

bors commented Dec 31, 2023

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #119066) made this pull request unmergeable. Please resolve the merge conflicts.

@GuillaumeGomez
GuillaumeGomez deleted the simplify-src-sidebar-js branch January 22, 2024 10:04
BlickandMorty added a commit to BlickandMorty/Epistemos that referenced this pull request Apr 26, 2026
…with Apple ld64 trap

Per docs/audits/EXTENDED_PROGRAM_PLAN_2026_04_25.md Wave 6
Cross-ref dpp §6.1-6.3 Sprint 5 + 6

Lands the canonical scaffolding for Wave 6's two production deliverables
per the Wave 6 research findings:
  1. PGO via cargo-pgo with the Apple ld64 lto="thin" workaround
  2. bumpalo per-frame arenas with the serde_json::RawValue copy trick

The actual MIGRATION work (rewriting MCP dispatch + render hot paths
to allocate from `with_frame`, running the full PGO sweep against the
Swift host) is follow-up — both require differential testing against
the existing allocator paths and a multi-day instrumentation cycle.

NEW agent_core/src/arenas/ module:
  - mod.rs               — module doc + re-exports
  - frame.rs             — thread_local Bump per worker thread
                            (canonical research finding: "one arena per
                            worker, NOT one global; Bump is !Sync by
                            design — sharing requires a Mutex that
                            destroys the win"). Exposes `with_frame(|arena| ...)`
                            which resets the bump pointer (O(1)) before
                            invoking the closure.
  - raw_value.rs         — `raw_value_in(&RawValue, &Bump) -> BumpString`
                            helper for the `serde_json` heap-string gotcha.
                            serde_json::from_str ALWAYS allocates owned
                            Strings on the heap regardless of arena;
                            the canonical workaround per the research is
                            to read once into &RawValue, then copy the
                            slice into the arena via BumpString::push_str.
                            Used by the MCP dispatch path migration.

Initial chunk capacity: 4 KB per thread (sized for the median MCP tool
call: dispatcher header + small payload). The arena grows on demand if
a single call exceeds this; bumpalo's `reset()` keeps the LARGEST chunk
across resets so steady-state has zero chunk-allocation churn.

NEW agent_core/Cargo.toml dependencies:
  - bumpalo 3.16 with `collections` feature (the canonical 2026 choice
    per research: typed-arena is single-type only; slab is for stable
    indices, wrong tool; no serious challenger has emerged)
  - serde_json `raw_value` feature enabled

NEW [profile.release-pgo] in agent_core/Cargo.toml:
  inherits = "release"
  lto = "thin"   ← critical Apple ld64 trap

  Apple's ld64 LTO pass discards `__llvm_prf_*` PGO instrumentation
  sections silently when lto = "fat". Verified against rust-lang/rust#119016.
  Thin LTO retains them. The canonical [profile.release] keeps lto = "fat"
  for non-PGO release artifacts; this separate profile is used only
  during the cargo-pgo cycle.

NEW scripts/pgo-cycle.sh:
  Drives one full cargo-pgo cycle for agent_core (the largest dylib at
  14 MB — biggest PGO leverage). Three subcommands: `instrument`,
  `optimize`, `all`. Embeds the Apple ld64 trap warning + reference to
  rust-lang/rust#119016 inline so a future maintainer reading the
  script understands why lto="thin" is non-negotiable here.

  Designed for manual PGO sweeps (NOT CI — instrumentation cycle takes
  ~20 minutes wall-clock). Documents that the bench alone is not enough
  coverage; the operator must drive the Swift host through real
  workloads (idle scroll, agent turn, graph layout) for 5+ minutes per
  the Wave 6 research finding ("3 distinct workloads totaling 5+ minutes;
  fewer over-fits to the dominant path and regresses cold paths").

8 new tests in agent_core arenas module:
  frame.rs (5):
    - frame_arena_resets_between_calls (address-equality verifies
      bump-pointer rewind across consecutive with_frame calls)
    - frame_arena_strings_can_be_built_inside_closure
    - frame_arena_handles_large_then_small_alloc_cycle (chunk reuse)
    - nested_with_frame_panics (leaf-frame contract enforcement)
    - separate_threads_have_independent_arenas (thread_local correctness)
  raw_value.rs (3):
    - raw_value_in_copies_bytes_into_arena
    - raw_value_in_handles_unicode
    - nested_raw_values_keep_their_serialised_form

6 new Swift source-guard tests in EpistemosTests/PGOAndArenasTests.swift:
  1. arenas/ module shape (mod.rs + frame.rs + raw_value.rs all exist)
  2. with_frame uses thread_local + arena.reset() per the research
  3. raw_value_in helper exists with BumpString
  4. agent_core/Cargo.toml declares bumpalo + serde_json raw_value
  5. [profile.release-pgo] uses lto = "thin" (Apple ld64 trap guard)
  6. pgo-cycle.sh exists, is executable, embeds the ld64/rust#119016 warning

Verification:
  - cargo test --release on agent_core arenas:: → 8/8 pass
  - cargo build --release --target aarch64-apple-darwin succeeds
  - All 6 PGOAndArenasTests pass via xcodebuild

Wave 6 follow-ups (per dpp §6.2-6.3, NOT in this commit):
  - Rewrite omega-mcp dispatcher to allocate scratch from with_frame
    (uses raw_value_in for the JSON-RPC body parse)
  - Bumpalo arena adoption in the Metal render driver path
  - Full PGO sweep + ≥5% wall-clock measurement against the dpp budget
  - xcrun xctrace report per dpp §8 acceptance criteria

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants