bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Optimize `validate_param`

Open MiniaczQ opened this issue 1 year ago • 3 comments

What problem does this solve or what need does it fill?

From discord discussion: https://discord.com/channels/691052431525675048/749335865876021248/1289681891611508809

As @cart pointed out, validate_param doubles lookup of get_param, which can have significant performance impact.

What solution would you like?

There are few potential ways of fixing that:

  1. Merge get_param with validate_param into try_get_param. 1.1. In naive approach, we always create a system task (multithreaded executor), even if parameters can't be retrieved. 1.2. We can try to run try_get_param before spawning a task, but then we need to somehow pass the parameters into the system. (type erasure issues)
  2. Keep get_param and validate_param, add try_get_param with default implementation that merges the previous 2. Implement manually for performance gains.

MiniaczQ avatar Sep 28 '24 21:09 MiniaczQ

I prefer solution 2 here.

alice-i-cecile avatar Oct 01 '24 16:10 alice-i-cecile

To summarize, the difficulty lies in the multithreaded case, as validation and fetching happen on different tasks: 1.1. Speeds up the case that the system runs at the expense of the case it doesn't. 1.2. ~~Type erasure isn't the issue (the system knows their type) – the problem is that they have the lifetime of the state & world used to fetch them, so we can't store them and hand them off to another task~~ 2. Does not help with the multithreaded executor. It also has the most code duplication.

While variant 1.2. is a bit tricky, it's best fit.

SpecificProtagonist avatar Oct 01 '24 18:10 SpecificProtagonist

All the presented solutions suffer from the same issue: combinator systems (which consist of many subsystems, exclusive ones included), can invalidate their own params a.pipe(b).pipe(a), b can remove params required for second a.

This means, we can never enforce that the entire System (defined by the trait) can run in it's entirety. If we drop this requirement and allow combinator systems to run "until they can't", then multithreaded executor only has to check whether the first subsystem can run.

This new solution is implemented in #15606

MiniaczQ avatar Oct 12 '24 15:10 MiniaczQ