Optimize / fix ScenarioConfigs.GetFullExecutionRequirements()
#2576
Labels
bug
evaluation needed
proposal needs to be validated or tested before fully implementing it in k6
performance
refactor
While reviewing #2573, I noticed some strange code in
ScenarioConfigs.GetFullExecutionRequirements()
, specifically this part:k6/lib/executors.go
Lines 318 to 321 in 556747f
It seems that
currentTimeOffset
is meaningless and we will always add all intermediate steps toconsolidatedSteps
, which is not ideal. It's probably just a minor performance problem and not a consequential bug becauseGetFullExecutionRequirements()
is not used for anything besides figuring out the max test VUs and duration at this point in time, but it could be a nasty surprise for us in the future if left unfixed.Digging into the history of the code, it seems like it's like this because of a bugfix (#1362) for another issue (#1358) from before we even release executors in k6 v0.27.0 🤯 Before that the code made a bit more sense, even if it was buggy:
k6/lib/executors.go
Lines 311 to 316 in 55a49d5
And I think the sorting change from #1362 and something like the
if
change from #2573 might also solve the original issue without having all of the confusing intermediate steps in the end result 🤔 And if we can't (which I doubt), we should at least better document the current behavior, so that the expectations of its users aren't broken.The text was updated successfully, but these errors were encountered: