Skip to content
This repository has been archived by the owner on Sep 20, 2021. It is now read-only.

Example benchmark #96

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

Conversation

dantleech
Copy link

@dantleech dantleech commented Sep 24, 2016

Just an example benchmark using PHPBench to get things started.

$ phpbench report --uuid=latest --report=hoa
benchmark: RulerBench, subject: benchAssert
+---------------------------------------------------------------+------+-----+------------+---------+---------+---------+--------+
| params                                                        | revs | its | mem_peak   | best    | mean    | mode    | rstdev |
+---------------------------------------------------------------+------+-----+------------+---------+---------+---------+--------+
| {"rule":"group in [\"customer\", \"guest\"] and points > 30"} | 10   | 10  | 2,249,760b | 3.786ms | 4.724ms | 4.123ms | 15.20% |
| {"rule":"points > 30"}                                        | 10   | 10  | 2,200,408b | 1.386ms | 1.836ms | 1.909ms | 17.00% |
| {"rule":"group in [\"customer\", \"guest\"]"}                 | 10   | 10  | 2,216,400b | 2.240ms | 2.781ms | 2.715ms | 15.34% |
+---------------------------------------------------------------+------+-----+------------+---------+---------+---------+--------+``

@@ -29,7 +29,8 @@
"hoa/visitor" : "~2.0"
},
"require-dev": {
"hoa/test": "~2.0"
"hoa/test": "~2.0",
"phpbench/phpbench": "^0.12.0"
Copy link
Author

Choose a reason for hiding this comment

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

This would be moved to hoa/test I suppose.

Copy link
Member

Choose a reason for hiding this comment

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

Totally agree. And we should also introduce a thin layer between PHPBench and Hoa, like we did for atoum, as Hoa\Test\Benchmark\Suite and this class will extend it. Thoughts?

Copy link
Author

@dantleech dantleech Sep 26, 2016

Choose a reason for hiding this comment

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

I think that makes sense (you could define f.e. setUp etc there too).

Copy link
Member

@Hywan Hywan left a comment

Choose a reason for hiding this comment

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

Excellent start, thank you! This is good basis for a discussion.

* @Iterations(10)
* @OutputTimeUnit("milliseconds")
*/
class RulerBench
Copy link
Member

Choose a reason for hiding this comment

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

I think we can rename RulerBench to Ruler since this is an Benchmark namespace.

Copy link
Author

Choose a reason for hiding this comment

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

Historically this was a mandatory suffix (like Test in PHPUnit), but not since 0.12 so, can do.

* Executes the method once (warmup) before measuring time.
*
* @Warmup(1)
* @BeforeMethods({"setUpAssert"})
Copy link
Member

Choose a reason for hiding this comment

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

Can you have a default setUp method?

Copy link
Author

Choose a reason for hiding this comment

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

You can have an abstract class which defines a setUp method (and its annotation)

* @BeforeMethods({"setUpAssert"})
* @ParamProviders({"provideRules"})
*/
public function benchAssert($params)
Copy link
Member

Choose a reason for hiding this comment

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

Can we rename the method, like case_assert? How do you detect a method representing a benchmark?

Copy link
Author

Choose a reason for hiding this comment

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

You can, but then you need to use the @Bench annotation.

Copy link
Member

Choose a reason for hiding this comment

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

So there is no way to say: “All methods matching this pattern are of kind bench”?

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

It sounds pretty cool!

@@ -29,7 +29,8 @@
"hoa/visitor" : "~2.0"
},
"require-dev": {
"hoa/test": "~2.0"
"hoa/test": "~2.0",
"phpbench/phpbench": "^0.12.0"
Copy link
Member

Choose a reason for hiding this comment

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

Totally agree. And we should also introduce a thin layer between PHPBench and Hoa, like we did for atoum, as Hoa\Test\Benchmark\Suite and this class will extend it. Thoughts?

@@ -0,0 +1,11 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

Can we move this file in a single place, like in Hoa\Test, it will avoid to have it inside each library?
Note that we will probably create the hoa test:bench or update the hoa test:run command, and thus I suppose we could link this phpbench.json file with an argument. We already do this with hoa test:run where we already compute the atoum command (https://github.com/hoaproject/Test/blob/f80f083a0d1cc7a044fdbca10ea46cdcdbef4c38/Bin/Run.php#L291-L326) with all the bootstrap file, configuration file etc.

Copy link
Author

Choose a reason for hiding this comment

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

The issue would be if you wanted to define f.e. a custom report.

However, most, if not all, of the configuration options can be specified at the CLI, so if you wrap it in hoa test:bench then we can ommit the phpbench.json file and include it only when the library has specific requirements.

Copy link
Author

Choose a reason for hiding this comment

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

We could also add a Hoa phpbench extension which programatically adds reports if required.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, sounds interesting. Can you tell me more about it please?

Copy link
Author

Choose a reason for hiding this comment

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

The Extension instances for PHPBench are like modules for the DI container. It should be possible to add new configurations and overwrite existing ones actually this is disallowed currently.

The extensions can be enabled via. the command line or in phpbench.json as with the example

Copy link
Member

@Hywan Hywan Sep 26, 2016

Choose a reason for hiding this comment

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

Would you like to give it a try? Please, feel free to say no!

@dantleech
Copy link
Author

Not currently, sounds like something that could easily be configured
though (i.e. assert_ instead of bench)

On Mon, Sep 26, 2016 at 05:20:18AM -0700, Ivan Enderlin wrote:

@Hywan commented on this pull request.


In [1]Test/Benchmark/RulerBench.php:

  •        ],
    
    •        [
      
    •            'rule' => 'points > 30'
      
    •        ],
      
    •    ];
      
    • }
    • /**
    • \* Benchmark parsing with rules of different complexity.
      
    • \* Executes the method once (warmup) before measuring time.
      
    • *
      
    • \* @Warmup(1)
      
    • \* @BeforeMethods({"setUpAssert"})
      
    • \* @ParamProviders({"provideRules"})
      
    • */
      
    • public function benchAssert($params)

So there is no way to say: “All methods matching this pattern are of kind
bench”?


You are receiving this because you authored the thread.
Reply to this email directly, [2]view it on GitHub, or [3]mute the thread.

Reverse link: [4]unknown

References

Visible links

  1. Example benchmark #96
  2. Example benchmark #96
  3. https://github.com/notifications/unsubscribe-auth/AAgZcQIOyBIeqWioLjZfmtK-jRt2hSjuks5qt7iCgaJpZM4KFmfz
  4. Example benchmark #96

@Hywan
Copy link
Member

Hywan commented Sep 26, 2016

It could be a good idea and a good feature!

@Hywan
Copy link
Member

Hywan commented Oct 14, 2016

@dantleech Am I a despot if I ask you to update this PR based on PHPBench 0.12.2?

@dantleech
Copy link
Author

@Hywan no problem, updated :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

2 participants