Closed andriidski closed 2 years ago
Thanks a lot for taking a look Mihai & David! Made all the suggested edits and kicked off a patch build here. I configured it with a diff to dsi
to include the feature flag for this expression in that variant, so I hope that should work
hey team! now that skunkworks is done, would appreciate a re-review, thanks!
I'm a little confused by the times. It looks like aggregation_read_commands is about 1 hour 18 minutes int he waterfall, but only 36 minutes here. What am I missing here.
Otherwise I did look at the numbers some. Some of the tests are returning less than 1 ops/s. Do we need to run those longer or cut down the test so they run faster? I'm concerned about noise in our results from running very few iterastions of the test.
hey david, thanks for the response. I'm not sure I totally follow about the aggregation_read_commands
taking longer in waterfall than here, it looked to me like everything ran as expected, though since this is my first performance related task, perhaps I am forgetting something. Regarding tests and the number, I did not see < 1 ops/s but perhaps then we can get rid of the 1000
"level" since as you pointed out the levels don't really tell anything particularly interesting. and even with just 10, 100 there should be enough tests given that the code generates sorts for strings, numbers, and objects
hey david, thanks for the response. I'm not sure I totally follow about the
aggregation_read_commands
taking longer in waterfall than here, it looked to me like everything ran as expected, though since this is my first performance related task, perhaps I am forgetting something. Regarding tests and the number, I did not see < 1 ops/s but perhaps then we can get rid of the1000
"level" since as you pointed out the levels don't really tell anything particularly interesting. and even with just 10, 100 there should be enough tests given that the code generates sorts for strings, numbers, and objects
I may be looking at the wrong things :) I was having a little bit of trouble figuring out which tasks to look at in your patch build. It looked like some of the new tests were in the aggregation_read_commands task. I'm in the office tomorrow, so the shortest path may be to look over someone's shoulder tomorrow (if someone else is in).
In the meantime, here's the aggregation_read_commands task in your patch on all feature flags: https://spruce.mongodb.com/task/performance_linux_wt_standalone_all_feature_flags_aggregation_read_commands_patch_c379649913d701e406bec73b51a85ea6b618f677_61aa62b7562343496401357b_21_12_03_18_34_18/tests?execution=0&sortBy=STATUS&sortDir=ASC
Ah phooey -- human error on my part on the times. I see now I must have looked at the wrong task for comparison (agg-query-comparison-read-commands, rather than aggregation-read-commands). Sorry about that bit.
I do still see Aggregation.Project.SortArray_ObjectArray_1000_Asc reporting 0.4636431077758186 as a result. So, that's one case less than 1. It is the 1000 level. What do Kyle and Mihai think about that one.
@dalyd, @dobroshynski -- it is a little sad that the perf is kind of slow that a 1k array can't be sorted... but I also think it's a bit of an extreme case. I'd be okay with dropping it?
The next slowest after the 1k array is Aggregation.Project.SortArray_ObjectArray_100_Desc
, which is at 6.914469750161532. I'm assuming that's in the stable zone?
@ksuarz that sounds fine to me.
thanks for the review & comments all. @dalyd David if you do not have any further objections / requests I will merge this in
Performance microbenchmarks for new $sortArray expression being added in PM-2552