Skip to content

Remove try-finally from @with_pool for inlining performance#32

Merged
mgyoo86 merged 15 commits intomasterfrom
perf/remove_try_finally
Mar 13, 2026
Merged

Remove try-finally from @with_pool for inlining performance#32
mgyoo86 merged 15 commits intomasterfrom
perf/remove_try_finally

Conversation

@mgyoo86
Copy link
Member

@mgyoo86 mgyoo86 commented Mar 13, 2026

Motivation

Julia cannot inline functions containing try-finally. This blocks the compiler from optimizing @inline @with_pool functions in hot loops.

Removing try-finally yields ~15-25% speedup on typical patterns (single/nested/multi-type), with larger gains on Julia 1.10 LTS. LLVM IR confirms the exception handler frame is completely eliminated.

What Changed

@with_pool now uses direct rewind insertion instead of try-finally:

  • rewind! inserted at every exit point: implicit return, explicit return, break, continue
  • Entry depth guard (while pool._current_depth > _entry_depth + 1) for deferred recovery when inner scope throws and outer catches
  • @goto safety: compile-time hard error on external jumps (set-difference analysis of goto targets vs local labels)

The old try-finally behavior is preserved as @safe_with_pool / @safe_maybe_with_pool.

API

Macro Strategy Inlinable Use case
@with_pool Direct rewind Yes Default — hot paths
@safe_with_pool try-finally No Exception safety required
@maybe_with_pool Direct rewind + runtime toggle Yes Conditional pooling
@safe_maybe_with_pool try-finally + runtime toggle No Exception-safe conditional

Exception Contract

Scenario @with_pool @safe_with_pool
Normal exit / return / break / continue Cleanup guaranteed Cleanup guaranteed
Inner throw + outer catch Deferred recovery via entry depth guard Immediate cleanup
Uncaught exception Pool corrupted (reset! needed) Cleanup guaranteed

mgyoo86 added 10 commits March 12, 2026 23:48
Prepare for try-finally removal from @with_pool by:
- Adding safe::Bool parameter to all 4 code generation functions
- Creating @safe_with_pool / @safe_maybe_with_pool (try-finally preserved)
- Adding GlobalRef constants for rewind functions (direct-rewind path)
- Exporting new macros

@safe_* macros pass safe=true, producing identical code to current
@with_pool (try-finally). No behavioral change yet — @with_pool still
uses try-finally until Phase 3 wires up the direct-rewind path.
- _generate_raw_rewind_call: un-escaped rewind Expr for AST embedding
- _generate_raw_entry_depth_guard: while-loop for leaked inner scope cleanup
- _transform_return_stmts: extended with optional rewind_call/entry_depth_guard
  kwargs; inserts guard+rewind before return (bare return included)
- _transform_break_continue: new walker for block-form pool scopes; skips
  :for/:while/:function/:-> bodies, transforms bare break/continue
- _check_goto_usage: compile-time warning when @goto found in pool scope

These functions are defined but not yet wired up — Phase 3 will connect
them to the code generators.
Replace try-finally with explicit rewind insertion at every exit point
(return, break/continue, implicit return) when safe=false (default).

- Extract _generate_block_inner and _generate_function_inner helpers
  to deduplicate logic across CPU and backend variants
- Block form applies _transform_break_continue; function form skips it
- Entry depth guard + validate→rewind order for deferred recovery
- safe=true preserves original try-finally behavior unchanged
… behavior

Document exception behavior contract: uncaught exceptions invalidate pool,
nested catch uses deferred recovery, PoolRuntimeEscapeError poisons pool.
Reference @safe_with_pool for try-finally guarantee.
- Fix entry_depth_var escaping bug (gensym must be esc'd to match
  references inside esc'd transformed_expr)
- Move exception safety test to @safe_with_pool (test_fallback_reclamation)
- Update expansion tests: assert no try-finally, verify _current_depth guard
- Add new tests: explicit return, break/continue, nested catch recovery,
  @safe_with_pool/@safe_maybe_with_pool exception safety
The previous _check_goto_usage checked for :symbolicgoto (lowered form)
but @goto/@Label appear as :macrocall at macro expansion time, so the
check never fired. Replace with _collect_local_gotos_and_labels which:
- Collects @goto targets and @Label names from the pool body AST
- Allows internal jumps (goto target has matching label in body)
- Hard errors on external jumps (goto target not in body = scope escape)
- Multiple internal @goto targets (fall-through paths)
- External @goto in function form (hard error)
- @goto inside inner lambda (correctly ignored)
- Mixed internal+external @goto (external wins → error)
- Multi-level nested leak recovery (2 inner scopes)
- Cross-type throw recovery (Int64 inner, Float64 outer)
- Uncaught exception → pool corrupted (documented limitation)
- @safe_with_pool handles uncaught exception correctly
The "Rewind on error" test expects cleanup after uncaught exception,
which requires try-finally. Switch from @with_pool to @safe_with_pool.
Verify CUDA rewind!/checkpoint! works correctly with macro-inserted
code: explicit return, break/continue, entry depth guard recovery,
and uncaught exception corruption (documented limitation).
Replace all try-finally references with direct-rewind path.
Add sections on exception behavior, entry depth guard, @safe_with_pool,
and exit point coverage table. Update code generation examples.
@codecov
Copy link

codecov bot commented Mar 13, 2026

Codecov Report

❌ Patch coverage is 95.45455% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.02%. Comparing base (f4298cb) to head (7a0469c).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
src/macros.jl 95.38% 6 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #32      +/-   ##
==========================================
- Coverage   97.20%   97.02%   -0.18%     
==========================================
  Files          14       14              
  Lines        2613     2658      +45     
==========================================
+ Hits         2540     2579      +39     
- Misses         73       79       +6     
Files with missing lines Coverage Δ
src/AdaptiveArrayPools.jl 100.00% <ø> (ø)
src/debug.jl 96.40% <100.00%> (+0.04%) ⬆️
src/macros.jl 98.10% <95.38%> (-0.69%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates AdaptiveArrayPools’ macro-based pooling scopes to use a direct-rewind (no try-finally) expansion for @with_pool / @maybe_with_pool to enable inlining and reduce overhead, while introducing exception-safe variants that retain try-finally behavior.

Changes:

  • Switch @with_pool / @maybe_with_pool to a direct-rewind expansion (inserts rewinds at exits + adds an entry-depth guard for leaked inner scopes).
  • Add @safe_with_pool / @safe_maybe_with_pool macros that use try-finally to guarantee cleanup on exceptions.
  • Update docs and tests (CPU + CUDA) to reflect the new exception behavior, @goto constraints, and expansion structure.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/macros.jl Implements direct-rewind codegen, entry-depth guard, @goto checks, and adds safe macros.
src/AdaptiveArrayPools.jl Exports the new @safe_* macros.
docs/src/architecture/macro-internals.md Documents the new direct-rewind strategy, exception behavior, and safe macro variants.
test/test_macros.jl Adds runtime tests for early return/break/continue, nested catch recovery, and safe macro exception cleanup.
test/test_macro_expansion.jl Updates macro-expansion assertions to reflect removal of finally in the direct-rewind path.
test/test_backend_macro_expansion.jl Updates backend macro-expansion assertions similarly (no finally, presence of entry-depth guard).
test/test_fallback_reclamation.jl Switches the exception-safety test to @safe_with_pool.
test/cuda/test_extension.jl Updates CUDA exception-safety test to @safe_with_pool and adds CUDA runtime verification for direct-rewind behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

mgyoo86 added 5 commits March 13, 2026 09:08
Insert _warn_leaked_scope() call before entry depth guard in both
block and function generators. @noinline to keep it off the hot path.
Zero cost at RUNTIME_CHECK=0 (dead code elimination).
- Entry depth guard: $rewind! → $_REWIND_REF for consistent GlobalRef usage
- AST walkers (_transform_return_stmts, _transform_break_continue,
  _collect_local_gotos_and_labels): skip :quote blocks to prevent
  quoted @Label from masking real external @goto
- Fix stale docstring referencing try-finally
- Add @safe_with_pool expansion test (retains try-finally)
- Add quoted @goto/@Label safety tests (false-negative + false-positive)
- Fix typed-rewind assertion to allow entry depth guard's rewind!(pool)
@mgyoo86 mgyoo86 merged commit dc45679 into master Mar 13, 2026
12 checks passed
@mgyoo86 mgyoo86 deleted the perf/remove_try_finally branch March 13, 2026 17:07
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