lucenenet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [lucenenet] NightOwl888 commented on pull request #349: Add separate benchmark.net project to run against code and not nuget packages
Date Fri, 25 Sep 2020 06:27:53 GMT

NightOwl888 commented on pull request #349:
URL: https://github.com/apache/lucenenet/pull/349#issuecomment-698747477


   Thanks for putting together this PR. We definitely need benchmarks in the master branch.
However, what we ought to shoot for are benchmarks that can be run by developers, added to
a nightly builds and release builds, and also plan for future expansion to benchmark individual
components of individual assemblies. I like the precedent set forth in [NodaTime.Benchmarks](https://github.com/nodatime/nodatime/tree/master/src/NodaTime.Benchmarks).
   
   A few things to consider:
   
   1. Since we don't have separate directories for `src` and `tests`, the build scripts rely
on naming conventions to determine which assemblies are test projects. Specifically, it relies
on the convention `Lucene.Net.Tests.<name>`. We shouldn't use that convention here,
as these will be handled differently in the build.
   2. We should have a top level directory for `benchmarks` (a sibling of `src`) so we can
expand to include benchmarks on each separate assembly (including the test framework). This
organization makes it possible to have `Directory.Build.props` and `Directory.Build.targets`
files that apply specifically to benchmarks.
   3. Within the `benchmarks` folder, we should have a separate `.sln` file that includes
all of the main assemblies and benchmarks, but not the tests.
   4. Some of the "tests" in Lucene are actually benchmarks as they only record the amount
of time the operation takes but don't actually have any asserts that can fail. These are wasting
time (about 5% of the total) during normal testing with little benefit. We should aim to remove
these from the tests and refactor them as benchmarks that run nightly and during releases.
Some examples:
      - [TestWordDelimiterFilter::TestPerformance()](https://github.com/apache/lucenenet/blob/ece6bea0a3c98961a77d7060b5615fccefabe725/src/Lucene.Net.Tests.Analysis.Common/Analysis/Miscellaneous/TestWordDelimiterFilter.cs#L35-L49)
      - [TestTeeSinkTokenFilter::Performance()](https://github.com/apache/lucenenet/blob/ece6bea0a3c98961a77d7060b5615fccefabe725/src/Lucene.Net.Tests.Analysis.Common/Analysis/Sinks/TestTeeSinkTokenFilter.cs#L178-L260)
      - [CacheSubSequencePerformanceTest](https://github.com/apache/lucenenet/blob/ece6bea0a3c98961a77d7060b5615fccefabe725/src/Lucene.Net.Tests.Analysis.Phonetic/Language/Bm/CacheSubSequencePerformanceTest.cs)
      - [PhoneticEnginePerformanceTest](https://github.com/apache/lucenenet/blob/ece6bea0a3c98961a77d7060b5615fccefabe725/src/Lucene.Net.Tests.Analysis.Phonetic/Language/Bm/PhoneticEnginePerformanceTest.cs)
      - [TestTermdocPerf](https://github.com/apache/lucenenet/blob/ece6bea0a3c98961a77d7060b5615fccefabe725/src/Lucene.Net.Tests/Index/TestTermdocPerf.cs)
   
   Of course, a key part of getting this into the build is to set up the instrumentation so
the benchmark results are easily available for viewing (ideally without having to download
the artifacts). This was dirt simple in TeamCity by including an HTML file as a tab in the
portal, but we haven't explored the options yet in Azure DevOps.
   
   We are running into a realistic limit of number of projects Visual Studio can load in a
reasonable timeframe, so ideally we would keep them in a separate `.sln`. This is the approach
that Microsoft takes to manage large projects.
   
   You don't need to include all of these changes in this PR, this is just to outline the
plan. But could you please set a precedent by putting the benchmarks in a top level `benchmarks`
directory and a `Lucene.Net.Benchmarks.sln` file to manage them?
   
   > If someone with experience with benchmarks in CI sees a flaw in this layout, please
do provide your valuable input.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



Mime
View raw message