344 introduced a performance issue with SynchronousEmitter (used in signal glitch propagation) when SynchronousSubscriptions are cancelled. Removal from a List of subscriptions was expensive and in the critical path for simulation performance when there are many Combinationals arranged in a particular way.
This PR replaces that List with a custom collection called IterableRemovableQueue which is optimized specifically for the needs of the SynchronousEmitter. It also includes a benchmark that reproduces the performance issue and tests for the new collection type.
There is still some overhead in having guard there at all, but without disabling the check there's not that much I see we can do to improve it. Moving towards recommending reduced usage of Combinational may be a good solution, since it has a number of weird characteristics (representing procedural code as hardware).
This PR also has some other refactoring of minor collection usage for efficiency based on discoveries during profiling.
Related Issue(s)
N/A
Testing
Existing tests pass, plus new ones for the benchmark and collection.
Backwards-compatibility
Is this a breaking change that will not be backwards-compatible? If yes, how so?
No
Documentation
Does the change require any updates to documentation? If so, where? Are they included?
Description & Motivation
344 introduced a performance issue with
SynchronousEmitter(used in signalglitchpropagation) whenSynchronousSubscriptions are cancelled. Removal from aListof subscriptions was expensive and in the critical path for simulation performance when there are manyCombinationals arranged in a particular way.This PR replaces that
Listwith a custom collection calledIterableRemovableQueuewhich is optimized specifically for the needs of theSynchronousEmitter. It also includes a benchmark that reproduces the performance issue and tests for the new collection type.There is still some overhead in having
guardthere at all, but without disabling the check there's not that much I see we can do to improve it. Moving towards recommending reduced usage ofCombinationalmay be a good solution, since it has a number of weird characteristics (representing procedural code as hardware).This PR also has some other refactoring of minor collection usage for efficiency based on discoveries during profiling.
Related Issue(s)
N/A
Testing
Existing tests pass, plus new ones for the benchmark and collection.
Backwards-compatibility
No
Documentation
No