Skip to content

Optimize: cache validated plugin arguments across reschedules - #10160

Open
bartv wants to merge 4 commits into
masterfrom
compiler-perf-check-args-cache
Open

Optimize: cache validated plugin arguments across reschedules#10160
bartv wants to merge 4 commits into
masterfrom
compiler-perf-check-args-cache

Conversation

@bartv

@bartv bartv commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

Summary

When a plugin is rescheduled after UnsetException, reuse previously validated ProcessedArgs instead of re-validating the same arguments. Type validation and domain conversion are deterministic for the same inputs.

Split from #10100.

Benchmark results (10 runs avg, dedicated benchmark machine)

Benchmark master with opt Delta
athonet_mpn 14.23s 14.67s +3.1%
connect_demo 7.78s 7.94s +2.1%
connect_infra 15.87s 16.19s +2.0%
inmanta_infra 14.55s 14.60s +0.3%
systemtenant 11.22s 11.32s +0.9%
Total 63.65s 64.72s +1.7%

Impact is within noise when isolated. This optimization primarily benefits models with many plugin reschedules (UnsetException). Its effect is visible in the synthetic compilerscaling benchmark with the plugin reschedule pipeline.

Test plan

  • CI passes

🤖 Generated with Claude Code

…t type validation after UnsetException

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@bartv bartv added the compiler label Mar 19, 2026
@bartv bartv self-assigned this Mar 19, 2026
@bartv
bartv requested a review from sanderr March 19, 2026 11:28
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread src/inmanta/ast/statements/call.py Outdated
Comment on lines 147 to 150
:param checked_args_cache: A mutable single-element list for caching validated plugin arguments across
reschedules. When provided, PluginFunction stores the result of check_args in checked_args_cache[0]
on the first call and reuses it on subsequent calls, avoiding redundant type validation.
"""

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.

I strongly dislike this pattern of mutating the argument. Is there no other way to cache it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Claude here, on behalf of Bart. Refactored — call_in_context and execute_call now accept Optional[CheckedArgs] as input and return Optional[CheckedArgs] as output. FunctionUnit stores the cache directly as Optional[CheckedArgs]. No more mutable argument mutation.

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.

I'm pretty sure that doesn't work because

  • we only need the cache in case of UnsetException
  • when we raise UnsetException we can not return the value (at least not in the current implementation.

I need to think on a proper solution.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Claude here, on behalf of Bart. You're right — the return-value pattern is broken for this use case. The flow:

  1. PluginFunction.call_in_context() computes processed_args and calls the plugin
  2. If UnsetException is raised, it's caught (line 311) and re-raised (line 324)
  3. return processed_args (line 334) is never reached
  4. In FunctionUnit.execute(), the re-raised UnsetException (a RuntimeException) is caught at line 396, so self._checked_args_cache is never assigned
  5. On reschedule, _checked_args_cache is still None

The cache is only useful when UnsetException is raised, but that's exactly when the return value is lost.

A correct approach would store the cache before the plugin call. Two options:

  1. Store on FunctionUnit directly: After check_args succeeds but before the plugin call, save processed_args on the FunctionUnit via a setter/attribute. On reschedule, pass it back. This avoids mutating the call signature but couples FunctionUnit to PluginFunction internals.

  2. Store via exception: Attach processed_args to the UnsetException when re-raising it, read it back in FunctionUnit.execute().

Given that the benchmark shows this optimization is neutral anyway (ARM v3: -1.3% total), the added complexity of either approach may not be justified. We'd suggest closing this PR unless you have a clean solution in mind.

Refactor checked_args caching to use return values instead of mutating
a list argument. call_in_context and execute_call now accept
Optional[CheckedArgs] as input and return Optional[CheckedArgs] as
output. FunctionUnit stores the cache directly.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants