Implemented power-management, add docs (#68) #952

MarekM25 · 2018-11-10T15:51:43Z

No description provided.

dnfclas · 2018-11-10T15:51:54Z

All CLA requirements met.

AndreyAkinshin · 2018-11-14T09:43:21Z

@MarekM25 thanks for the PR.

AndreyAkinshin · 2018-11-14T09:44:16Z

docs/articles/configs/powerplans.md

+
+# Power Plans
+
+This description concerns only version v0.11.2 and above of BenchmarkDotNet. In the previous versions, benchmarks in the power plans.


We don't need a note about the BenchmarkDotNet version. The documentation is always relevant only for the latest version of BenchmarkDotNet.

AndreyAkinshin · 2018-11-14T09:45:46Z

samples/BenchmarkDotNet.Samples/IntroPowerPlan.cs

+namespace BenchmarkDotNet.Samples
+{
+    [HighPerformancePowerPlan(false)]
+    public class IntroPowerPlan


In the IntroSample, we need two jobs (enabled and disabled high performance power plan) to show the difference between them. An example of the summary table should be included in IntroPowerPlan.md.

AndreyAkinshin · 2018-11-14T09:47:45Z

src/BenchmarkDotNet/Attributes/HighPerformancePowerPlanAttribute.cs

+    /// determines if BenchmarDotNet changes power plan to High Performance
+    /// </summary>
+    [PublicAPI]
+    public class HighPerformancePowerPlanAttribute : Attribute, IConfigSource


It makes sense to add [AttributeUsage(AttributeTargets.Class | AttributeTargets.Assembly, AllowMultiple = true)] here. It will allow comparing enabled/disabled configurations.

To be sure I thought about only enabling/disabling the high-performance plan. From my perspective, allowing multiple attributes with the current implementation is not enough (of course, I might be wrong). However, it is a remarkably great idea to do it, and I suggest doing more here. I mean not only analyzing disabled and enabled a high-performance plan but making it possible to compare any power plan for a user. For example, we could compare HighPerformance, Balanced, PowerSaver. What is more, I consider allowing a user to both use power plan enum and provide it by GUID string. The first way contains popular power plan GUIDs such as I mentioned previously. The second approach is essential because companies could have own, custom power plans for any device. This feature allows targeting benchmarks when we could expect what power plan is used by an end user. We could benchmark different power plans if we know that our software will be run on a server, a personal laptop or mobile phone. I could do it willingly as my next issue or by continuing actual:) What do you think?

AndreyAkinshin · 2018-11-14T09:49:30Z

src/BenchmarkDotNet/Running/PowerManagementApplier.cs

+    internal class PowerManagementApplier
+    {
+        private Guid? _userCurrentPowerPlan;
+        private bool _powerPlanChanged = false;


We don't use underscores as private field prefixes. Please, remove it.

AndreyAkinshin · 2018-11-14T09:50:23Z

tests/BenchmarkDotNet.IntegrationTests/PowerManagementApplierTests.cs

+        [Fact]
+        public void TestSettingAndRevertingBackGuid()
+        {
+            if (RuntimeInformation.IsWindows())


Please, use FactWindowsOnlyAttribute instead of such checks.

Thx, this is exactly what I needed ! :)

AndreyAkinshin · 2018-11-14T10:05:17Z

src/BenchmarkDotNet/Running/PowerManagementApplier.cs

+        {
+            if (RuntimeInformation.IsWindows())
+            {
+                if (highPerformancePowerPlan && _powerPlanChanged == false)


Could we move the Apply logic from constructor to a separate method?

Could you tell me what do you mean by that? It is not the constructor but a method. Perhaps, I don't understand you. However, I moved logger injection to the constructor.

# Conflicts: # src/BenchmarkDotNet/Configs/ManualConfig.cs

MarekM25 · 2018-12-15T20:22:27Z

@MarekM25 thanks for the PR.

And thanks for your review! :) Could you look why my build fail? Perhaps, we should retry it. I couldn't find that option was available for me.

AndreyAkinshin · 2018-12-16T09:06:29Z

@MarekM25 the build is restarted.

To be sure I thought about only enabling/disabling the high-performance plan. From my perspective, allowing multiple attributes with the current implementation is not enough (of course, I might be wrong). However, it is a remarkably great idea to do it, and I suggest doing more here. I mean not only analyzing disabled and enabled a high-performance plan but making it possible to compare any power plan for a user. For example, we could compare HighPerformance, Balanced, PowerSaver. What is more, I consider allowing a user to both use power plan enum and provide it by GUID string. The first way contains popular power plan GUIDs such as I mentioned previously. The second approach is essential because companies could have own, custom power plans for any device. This feature allows targeting benchmarks when we could expect what power plan is used by an end user. We could benchmark different power plans if we know that our software will be run on a server, a personal laptop or mobile phone. I could do it willingly as my next issue or by continuing actual:) What do you think?

It looks pretty interesting. However, let's do the minimal implementation first. Currently, HighPerformancePowerPlan is a property of a config. That's why you had to duplicate your benchmark in the sample:

    [HighPerformancePowerPlan(false)]
    public class IntroPowerPlanDisabled
    {
        [Benchmark]
        public int SplitJoin()
            => string.Join(",", new string[1000]).Split(',').Length;
    }
    // By default benchmark.net uses high-performance power plan.
    // There is no need to set it to true explicitly
    [HighPerformancePowerPlan(true)]
    public class IntroPowerPlanEnabled
    {
        [Benchmark]
        public int SplitJoin()
            => string.Join(",", new string[1000]).Split(',').Length;
    }

The PowerPlan should be a property of a job (like Platform, Jit, and so on). In this case, users will be able to apply different power plans to the same benchmark class without code duplication.

Cosmetic fixes (dotnet#68)

MarekM25 · 2019-02-01T19:12:10Z

Hi @AndreyAkinshin ,
I finished working on this PR. Could you have a look? What do you think?
The build failed, but the same test locally and before squashing commits works fine. Is it possible that the test is unstable?

AndreyAkinshin · 2019-02-02T08:01:14Z

@MarekM25 I will review this PR on the next week. Meanwhile, could you please resolve conflicts with the master branch?

# Conflicts: # src/BenchmarkDotNet/Configs/ConfigExtensions.cs

MarekM25 · 2019-02-05T19:43:52Z

@MarekM25 I will review this PR on the next week. Meanwhile, could you please resolve conflicts with the master branch?

Thx, done ;) @AndreyAkinshin could you take a look? :)

# Conflicts: # src/BenchmarkDotNet/Jobs/EnvironmentMode.cs

AndreyAkinshin · 2019-04-01T17:26:25Z

@MarekM25 sorry for such a delay, I had to verify this branch on different laptops. It works great! Thanks for your contribution!

MarekM25 · 2019-04-01T21:26:33Z

@MarekM25 sorry for such a delay, I had to verify this branch on different laptops. It works great! Thanks for your contribution!

Thx, it was pleasure for me!

Implemented power-management, add docs (dotnet#68)

6ab4270

AndreyAkinshin reviewed Nov 14, 2018

View reviewed changes

MarekM25 added 2 commits December 5, 2018 23:00

Power Management review changes (dotnet#68)

526526a

Merge branch 'master' into power-management

2dd626c

# Conflicts: # src/BenchmarkDotNet/Configs/ManualConfig.cs

MarekM25 force-pushed the power-management branch from 9623f14 to 684e99d Compare January 13, 2019 22:57

Comparing power plans (dotnet#68)

f3a51c2

Cosmetic fixes (dotnet#68)

MarekM25 force-pushed the power-management branch from 4a9e823 to f3a51c2 Compare January 31, 2019 21:44

Merge branch 'master' into power-management

897bfb8

# Conflicts: # src/BenchmarkDotNet/Configs/ConfigExtensions.cs

Merge branch 'master' into power-management

04f79a5

# Conflicts: # src/BenchmarkDotNet/Jobs/EnvironmentMode.cs

AndreyAkinshin merged commit 8aa6ade into dotnet:master Apr 1, 2019

AndreyAkinshin added this to the v0.11.5 milestone Apr 1, 2019

AndreyAkinshin mentioned this pull request Apr 1, 2019

Power management #68

Closed

MarekM25 mentioned this pull request Apr 14, 2019

The power management feature extension #1132

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented power-management, add docs (#68) #952

Implemented power-management, add docs (#68) #952

MarekM25 commented Nov 10, 2018

dnfclas commented Nov 10, 2018 •

edited

Loading

AndreyAkinshin commented Nov 14, 2018

AndreyAkinshin Nov 14, 2018

AndreyAkinshin Nov 14, 2018

AndreyAkinshin Nov 14, 2018

MarekM25 Dec 15, 2018

AndreyAkinshin Nov 14, 2018

AndreyAkinshin Nov 14, 2018

MarekM25 Dec 15, 2018 •

edited

Loading

AndreyAkinshin Nov 14, 2018

MarekM25 Dec 15, 2018

MarekM25 commented Dec 15, 2018

AndreyAkinshin commented Dec 16, 2018

MarekM25 commented Feb 1, 2019

AndreyAkinshin commented Feb 2, 2019

MarekM25 commented Feb 5, 2019 •

edited

Loading

AndreyAkinshin commented Apr 1, 2019

MarekM25 commented Apr 1, 2019


		# Power Plans

		This description concerns only version v0.11.2 and above of BenchmarkDotNet. In the previous versions, benchmarks in the power plans.

Implemented power-management, add docs (#68) #952

Implemented power-management, add docs (#68) #952

Conversation

MarekM25 commented Nov 10, 2018

dnfclas commented Nov 10, 2018 • edited Loading

AndreyAkinshin commented Nov 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarekM25 Dec 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarekM25 commented Dec 15, 2018

AndreyAkinshin commented Dec 16, 2018

MarekM25 commented Feb 1, 2019

AndreyAkinshin commented Feb 2, 2019

MarekM25 commented Feb 5, 2019 • edited Loading

AndreyAkinshin commented Apr 1, 2019

MarekM25 commented Apr 1, 2019

dnfclas commented Nov 10, 2018 •

edited

Loading

MarekM25 Dec 15, 2018 •

edited

Loading

MarekM25 commented Feb 5, 2019 •

edited

Loading