Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated fennecs to 0.4.0-beta #36

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

thygrrr
Copy link

@thygrrr thygrrr commented May 12, 2024

Hello!

I'm Tiger, the maintainer of 🦊 fennecs. Thanks for making this nice Benchmark repo, and thanks so much to @xentripetal for building the fennecs benchmarks. And I'm also a big fan of DefaultEcs. ^^

Content of this PR

Framework

  • Program.cs now allows a custom #define RANK_RESULTS, which adds an Orderer to the Config. While iterating, developers can now more clearly see where their fast/slow methods are at in relation to others.
  • it was added as an optional CLI argument so as not to change any existing output of the suite
  • the Extension WithCapabilityExclusions(this IConfig) was written and Program.cs uses it at startup
  • a CategoryExclusion : IFilter was created and documented - if this filter is present, it will turn off any categories it matches. (it supports a single string)
  • this decision of whether to add the filter to turn off affected categories happens in WithCapabilityExclusions
  • the class Capabilities in Categories.cs was created
    • it provides convenient Category string constants that Benchmark writers can add to their Benchmarks to ensure these benchmarks *only run if the capability is present.
    • other benchmarks are unaffected
    • example: Benchmarks in Category Capabilities.AdvSIMD will only show up on appropriate Arm64 platforms.
    • This means incompatible Benchmarks don't need to fail (causing spam) or return early (thus scoring very high)
  • a subset of common vector instruction sets were added to the Capabilities class (but not all of them - didn't want to bloat)
  • several Benchmarks now have their own name added to their categories. (using nameof to stay robust with refactors)
    • SystemWithTwoComponentsMultipleComposition
    • SystemWithOneComponents
    • SystemWithTwoComponents
    • SystemWithThreeComponents
    • this makes it easier to directly target them from the command line

fennecs 0.4.0-beta support

  • affects all three entity creation benchmarks for fennecs
  • updates the dependency from fennecs 0.1.1-beta to the recent release, fennecs 0.4.0-beta
  • changed project link on README.MD
  • had to remove hardcoded chunk sizes (since 0.3.5 uses an internal concurrency heuristic, in 0.5.0 concurrency will be user-configurable again)
  • optimized the fennecs benchmarks in Category.System, and wrote additional ones:
    • Query<>.Raw Benchmarks now have x64 AVX2, SSE2, and Arm64 AdvSIMD support, with comments
    • delegate pattern replaced with anonymous static methods (delegates still work, just wanted consistency)
    • fennecs Benchmarks have clearer Benchmark Descriptions
  • changed spelling of "fennecs" in categories and comments
  • please let me know if the fennecs (Blit) benchmark bends the rules too far... 🗡️

Have a good day, and I'm looking forward to your review/feedback.

@thygrrr thygrrr changed the title Updated Fennecs 0.3.5 beta Updated fennecs to 0.3.5 beta May 12, 2024
@thygrrr thygrrr mentioned this pull request May 12, 2024
6 tasks
Comment on lines 40 to 45
if (args.Contains("--ranking"))
{
List<string> argList = args.ToList();
argList.Remove("--ranking");
args = argList.ToArray();
configuration = configuration.WithOrderer(new DefaultOrderer(SummaryOrderPolicy.FastestToSlowest));
Copy link
Owner

@Doraku Doraku May 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this is an active or not settings in benchmarkdotnet. I'm thinking that maybe it would be better to stick to their philosophy (and what I'm doing for the CHECK_CACHE_MISSES define and HardwareCounters attribute for exemple 🤔). What do you think?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can go with the #define (to make starting conditions more uniform), but I'd still use it at the configuration level, rather than at the Attribute level. (BenchmarkDotNet encourages both).

Working on it right now..

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IConfig configuration = DefaultConfig.Instance
    .WithOptions(ConfigOptions.DisableOptimizationsValidator)
    .WithCapabilityExclusions();

#if RANK_RESULTS
    configuration = configuration.WithOrderer(new BenchmarkDotNet.Order.DefaultOrderer(BenchmarkDotNet.Order.SummaryOrderPolicy.FastestToSlowest));
#endif

if (args.Length > 0)
{
    benchmark.Run(args, configuration);
}
else
{
    benchmark.RunAll(configuration);
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know what you think, otherwise I can just remove the change entirely. For my own workflow, the define is certainly less convenient than a command line parameter, but it's your repo and you also run the benchmark in its full scale, so this is what counts.

…ities.cs, and changed the --ranking option to a RANK_RESULTS define
@thygrrr thygrrr changed the title Updated fennecs to 0.3.5 beta Updated fennecs to 0.4.0-beta May 22, 2024
@thygrrr thygrrr closed this May 22, 2024
@thygrrr thygrrr deleted the fennecs-0.3.5-beta branch May 22, 2024 18:45
@thygrrr thygrrr restored the fennecs-0.3.5-beta branch May 22, 2024 18:46
@thygrrr
Copy link
Author

thygrrr commented May 22, 2024

My bad, I renamed the branch on my repo :D

@thygrrr thygrrr reopened this May 22, 2024
@martindevans martindevans mentioned this pull request Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants