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

Refactoring the roslyn dependency into another package #225

Closed
ig-sinicyn opened this issue Jul 7, 2016 · 5 comments
Closed

Refactoring the roslyn dependency into another package #225

ig-sinicyn opened this issue Jul 7, 2016 · 5 comments
Milestone

Comments

@ig-sinicyn
Copy link
Contributor

The BDN 0.9.8 includes a dependency on roslyn compiler package. This results in 42 additional nuget packages, 300+ mb of binaries in the packages folder and 17 (or so) additional assembly references.

As for me it's too big overhead for the single library:)
And our perf tests project uses custom toolchain and therefore do not depend on roslyn at all.

Can the dependency be refactored into separate opt-in package?

@adamsitnik
Copy link
Member

All options are posted here: #149 (comment)

What it gives us is managed, type safe api that allows us to compile C# apps not only on Windows but also also Unix and MacOC with Mono. So great gain, but not for free.

We could make it a package, but then it would complicate the usage for the end-users.

As for now I can's see a good solution. But I understand your point @ig-sinicyn

@AndreyAkinshin
Copy link
Member

@adamsitnik, we can do the following:

  • Add a package BenchmarkDotNet.Core which will include all infrastructure stuff.
  • Add a package BenchmarkDotNet.Toolchains.Roslyn which will include all Roslyn stuff + dependencies.
  • Add reference from BenchmarkDotNet to these two packages.

So, @ig-sinicyn will be able to use BenchmarkDotNet.Core with own custom toolchain, other end-users will be able to use BenchmarkDotNet (with two new dependencies) without any additional troubles.

@adamsitnik
Copy link
Member

Ok, done. The disadvantage is that BenchmarkRunner and Switcher had to stay as part of BechmarkDotNet (not .Core) due to dependency to RoslynToolchain (unknown in BenchmarkDotNet.Core). Maybe there is some better solution, but I could not find it for this moment ;.

image

image

@adamsitnik adamsitnik changed the title [Suggestion] Please, refactor the roslyn dependency into another package Refactoring the roslyn dependency into another package Jul 18, 2016
@ig-sinicyn
Copy link
Contributor Author

@adamsitnik, as proposal:

What if the core logic of the BenchmarkRunner will be placed in .Core project in separate class and the BenchmarkRunner will just call its methods passing default toolchain as a call arg?

I've did something like this as we have nUnit/xUnit/MsTest support, console runner and also self-testing runner.

adamsitnik added a commit that referenced this issue Jul 18, 2016
@adamsitnik
Copy link
Member

@ig-sinicyn Good idea! I have implemented it right away, now you have BenchmarkRunnerCore that has single method exposed to BenchmarkRunner which can be also reused by you.

The definition:
internal static Summary Run(Benchmark[] benchmarks, IConfig config, Func<IJob, IToolchain> toolchainProvider)

Sample usage can be following:
BenchmarkRunnerCore.Run(benchmarks, config, job => job.Toolchain);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants