I hate to just close what’s there but it also likely needs some massaging. 
Additions to the benchmarks risk destabilizing the CI if the benchmark has a 
wide variance. I don’t have time right now to spend on tuning new benchmarks. 
If someone else does please step up and take it on.


> On Jan 22, 2020, at 12:31 PM, Donal Evans <[email protected]> wrote:
> 
> Two of those PRs are mine, so perhaps I can give a bit of context for
> people who might look at them. The oldest of the two, "Feature/Add PdxType
> benchmark and additional framework flexibility" was an attempt to quantify
> and maintain the improvement in performance for PdxType creation when large
> numbers of PdxTypes already exist, and to allow the passing of additional
> system properties to the VMs hosting the servers in order to change the log
> level and prevent the benchmark measuring how long it takes to log PdxType
> creation rather than actual time taken to create new PdxTypes. This PR has
> been open for a very long time, so it's possible that the changes regarding
> passing additional system properties to the VMs are now outdated or
> unnecessary, but the actual benchmarks themselves still have some value.
> 
> The second PR, "Added benchmarks for aggregate functions" contains 16 new
> benchmarks related to aggregate OQL queries, (8 each for Partitioned and
> Replicated regions), which were added following work in that area by the
> Commons team. The build is currently marked as failing, but this is due to
> a timeout rather than an actual build failure, as the number of benchmarks
> added increased the total time to build beyond the currently configured
> timeout. Adding such a large number of additional benchmarks will probably
> also noticeably increase the time it takes benchmarks to run, which bears
> consideration.
> 
> I hope this helps shed some light for people who may look over those PRs.
> 
> On Wed, Jan 22, 2020 at 11:36 AM Dan Smith <[email protected]> wrote:
> 
>> Hi,
>> 
>> I noticed we have some old outstanding PRs for the geode-benchmarks
>> project. Are any of these things we want to merge or should we close them
>> out?
>> 
>> https://github.com/apache/geode-benchmarks/pulls
>> 
>> -Dan
>> 

Reply via email to