-
-
Notifications
You must be signed in to change notification settings - Fork 969
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
Basic BenchmarkDotNet support for Xamarin #1360
Conversation
40655b9
to
4553542
Compare
Questions:
|
@adamsitnik or @AndreyAkinshin any thoughts about this? thanks! |
Hi! Any updates on this? BR |
@adambarath I never heard anything back on this. I'll use my best judgement to make this "mergeable" and maybe we'll hear back by then? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall the changes introduced to the main part of BenchmarkDotNet look very good to me 👍
It's great that you have added samples, but I would prefer to keep them in a separate solution. It could be called BenchmarkDotNet.Xamarin
. My main motivation is to make sure that other contributors don't need to install any new prerequisites to be able to build main solution. We also don't need to introduce any big changes to our CI.
@AndreyAkinshin do you agree on having a standalone solution for the Xamarin purposes?
548b27c
to
f8f8703
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Big thanks for adding the docs!
samples/BenchmarkDotNet.Samples.Forms/BenchmarkDotNet.Samples.Forms.csproj
Outdated
Show resolved
Hide resolved
* Multi-target `BenchmarkDotNet.Samples` to include `netstandard2.0` * Don't probe for `mono` binary for `InProcess` * Detect Xamarin.Android / Xamarin.iOS * Fix for `Directory.CurrentDirectory()` as `ArtifactPath` * Default to `InProcess` for Xamarin platforms * Added documentation, sample, and a new `BenchmarkDotNet.Xamarin.sln`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the documentation hasn't been deployed yet |
Fixes: #1343
This is basically an "MVP" of Xamarin support with BenchmarkDotNet.
At a high level:
Simple benchmarks work with default config settings.
You can write a Xamarin.Forms app that runs benchmarks and displays
the results. I added a very basic sample of this.
Some of the details to get this working:
ConsoleExitHandler.CancelKeypress
throwsPlatformNotSupportedException
on Xamarin platforms. Just ignorethis exception.
Multi-target
BenchmarkDotNet.Samples
to includenetstandard2.0
.This allows me to reference the benchmarks from the Xamarin.Forms
shared code. I had to exclude some Windows-specific benchmarks.
Don't probe for
mono
binary forInProcess
. This was breakingInProcess
for Xamarin, in general. It's not going to find amono
binary that doesn't exist.
Detect Xamarin.Android / Xamarin.iOS. I used the most common
Type.GetType
lookup to detect each platform:Java.Lang.Object
and
Foundation.NSObject
.Fix for
Directory.CurrentDirectory()
asArtifactPath
. Thecurrent directory on Android is not writeable. Looking for
/
,seemed like a simple fix for Android.
Default to
InProcess
for Xamarin platforms. Since we can't find amono
binary, we have to run in-process. Xamarin.iOS also shouldn'tuse
System.Reflection.Emit
until the interpreter is more stable.Users of BDN can experiment with the interpreter by configuring
their benchmarks manually.
This is the first I've looked at this repo, so suggestions welcome! I don't
know this codebase.
The bulk of the new files here is what you get from a new Xamarin.Forms
template in Visual Studio. We can trim that down if needed. Thanks!