Skip to content

add possibility to specify env vars via console line arguments#1313

Merged
adamsitnik merged 2 commits intomasterfrom
envVarsArgs
Nov 19, 2019
Merged

add possibility to specify env vars via console line arguments#1313
adamsitnik merged 2 commits intomasterfrom
envVarsArgs

Conversation

@adamsitnik
Copy link
Member

{
baseJob = baseJob.With(options.EnvironmentVariables.Select(text =>
{
var separated = text.Split(EnvVarKeyValueSeparator);
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps text.Split(EnvVarKeyValueSeparator, 2) to allow : to be used in the value?

@filipnavara
Copy link
Member

Thank you very much!

@adamsitnik adamsitnik merged commit e92d6d6 into master Nov 19, 2019
@adamsitnik adamsitnik deleted the envVarsArgs branch November 19, 2019 19:25
@adamsitnik
Copy link
Member Author

@filipnavara thanks for the review! The 0.12.0.1234 version contains new feature. You can download it from BDN CI feed https://benchmarkdotnet.org/articles/guides/nuget.html#nightly

@filipnavara
Copy link
Member

filipnavara commented Nov 19, 2019

Many thanks, @adamsitnik! Now I just need to convince someone to update the version dependency in dotnet/performance. I'm more than happy to submit a PR but I have been told previously that it may not be accepted until someone reviews all BDN changes picked in the update.

Update: I see the CI feed was removed in favor of the official 0.12 release. I'm fine with patching it locally for my builds for the time being but in longer term I would love to remove any unnecessary fragile patches from my build steps.

@adamsitnik
Copy link
Member Author

We have removed the dependency to BDN CI feed from dotnet/performance repo after BDN 0.12.0 was shipped to nuget.org. Adding it back again would be hard due to security reasons. We also don't plan to ship a new version of BDN to nuget.org in the next few weeks, so you should most probably keep using your fork|branch.

Is there anything else that needs to be changed in BDN to make the new mono from dotnet/runtime work? If supporting new Mono requires some changes we might consider shipping new BDN much sooner that we were planning.

@filipnavara
Copy link
Member

Is there anything else that needs to be changed in BDN to make the new mono from dotnet/runtime work?

Short answer: Hopefully not.

From the perspective of BDN Mono looks just like .NET 5 runtime. The exact integration into the build system is not complete yet but hopefully it won't require any major changes.

The scripts I use are public (https://github.com/filipnavara/mono-performance/) and it was running for few months now. I am now trying to reduce any custom steps in the build process to avoid potential breakage of the automated process. There were two custom changes applied on dotnet/performance in the process. One of them was the hack for environment variables that is now resolved with this PR. The other one is a patch for more graceful fallback in case libgdiplus is not installed on the machine (to be removed; loading native libraries was broken for few weeks in Mono due to refactoring to properly support .NET Core 3 APIs).

The only remaining issue is that there are certain benchmarks that depend on HW intrinsics. Availability of these HW intrinsics varies between runtimes and platforms. In case they are not available on given platform the benchmarks bail out early with nearly zero running time. This sticks out when comparing the results because Mono looks infinitely faster than CoreCLR on those benchmarks.

@AndreyAkinshin AndreyAkinshin added this to the v0.12.1 milestone Nov 20, 2019
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.

Add an option to pass environment variables to the default job

3 participants