On 18/10/2018 15:23, Artem Pisarenko wrote: >> We can also move the switch statement to a separate function, it >> simplifies the code: >> ... > > When I prepared this patch my intuition said me to add note in advance: > "Paolo, please, don't try to move this to a separate function. I've > tried it already. It cannot be done correct, look nice and not decrease > performancy at the same time". But I've ignored it... :) > Change you did is correct and nice, but (compared to my version) it adds > extra unlock/lock pair for running each timer list when it isn't empty > and in non-rr mode (where we would ignore checkpoints and execution flow > would bypass whole "if (need_replay_checkpoint) {...}" block). > Maybe you're aware of it, but I don't think that such change worth it.
No, you're right. The if should remain in the caller, or need_replay_checkpoint must be initialized with replay_mode. Paolo