perf(allocator): mark ReplaceWith panic path cold

[camc314]

Summary

Mark ReplaceWith's write_dummy helper as cold.

The helper is only reached while unwinding from a panic in a replacer closure. Telling the optimizer that this path is cold helps keep its allocator and dummy-construction code out of hot code layout and improves inlining decisions in unwind builds.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[codspeed-hq]

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 9 skipped benchmarks¹

---

_{Comparing codex/cold-replace-with-panic-path (64d70f7) with main (ca4a4a7)}

Footnotes

1. 9 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[overlookmotel]

I don't think this makes any difference. #[inline(never)] is already present, for this exact reason.

And, as far as I know, #[cold] primarily affects branch layout. But in this case, there's no branch to hint - write_dummy is called from drop unconditionally.

But why ReplaceWith seems to have had a small negative perf effect in transformer (at least according to Codspeed) is a mystery to me, so I may well be wrong.

What is Mr Codex telling you?

[overlookmotel]

Oh actually, I'm pretty sure this cannot account for the perf drop. Our benchmarks are built with panic = "abort", so all the drop guard code is deleted by compiler anyway. Whether it's marked #[cold] or not is irrelevant to the perf change - it must be, because that code isn't there at all in the assembly.

Whether it's a good idea regardless, I don't fully know. I don't think so. But I imagine you have info that contradicts my assumptions, which motivated this PR. I'd like to know what it is!

[camc314]

Whether it's a good idea regardless, I don't fully know. I don't think so. But I imagine you have info that contradicts my assumptions, which motivated this PR. I'd like to know what it is!

See my msg in Gchat for the actual replace with perf problem.

This was unrelated that i noticed, from my (admittedly limited understanding)

For cold:

Place it in a separate “cold” section of the binary.

• Optimize for smaller code rather than speed.
• Assume branches leading to it are unlikely.
• Improve instruction-cache locality for the hot path.

whereas inline never just discourages inlining.

But, both of these are just suggestions to the compiler!

[overlookmotel]

Merge activity

• Jul 15, 3:38 PM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
• Jul 15, 3:38 PM UTC: overlookmotel added this pull request to the Graphite merge queue.
• Jul 15, 3:44 PM UTC: The Graphite merge queue couldn't merge this PR because it was not satisfying all requirements (Failed CI: 'Test NAPI Compiler').