Skip to content

Convert performance tests to PHPBench#248

Merged
Ocramius merged 1 commit intodoctrine:masterfrom
Majkl578:phpbench
Feb 26, 2019
Merged

Convert performance tests to PHPBench#248
Ocramius merged 1 commit intodoctrine:masterfrom
Majkl578:phpbench

Conversation

@Majkl578
Copy link
Copy Markdown
Contributor

@Majkl578 Majkl578 commented Feb 25, 2019

Instead of testing perf with PHPUnit and printing some fancy output, we can use PHPBench instead.

It's not possible to have phpbench/phpbench as dev-dependency due to back-reference to doctrine/annotations 1.x.

I don't have much experience with writing PHPBench suites yet so suggestions are welcomed. :)

Comment thread .travis.yml
- stage: Benchmark
install:
- travis_retry composer update --prefer-dist
- curl -o phpbench.phar https://phpbench.github.io/phpbench/phpbench.phar
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I know I'm a weirdo, but I'd rather composer require with a specific version here. I don't consider curl to be a good way to install things...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'd too, but it's not possible, as said above:

It's not possible to have phpbench/phpbench as dev-dependency due to back-reference to doctrine/annotations 1.x.

Also curling specific version is currently not possible as versioned PHARs are not published (already discussed with @dantleech to be possible in future).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Otherwise a phar could be downloaded with phive, but I had a few issues with travis in the past.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Still same issues: would need to be in the dependency graph. Let's tackle the problem once we hit 2.0.0 (stable) then, and a dependency loop can be re-introduced, if needed.

Comment thread composer.json
"Doctrine\\Tests\\Annotations\\": "tests/Doctrine/Tests/Annotations"
},
"files": [
"tests/Doctrine/Tests/Annotations/Fixtures/SingleClassLOC1000.php"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this need to be in classmap?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's an unnamespaced class that was loaded using require_once previously.

Comment thread phpbench.json.dist
@@ -0,0 +1,4 @@
{
"bootstrap": "vendor/autoload.php",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Possibly not needed if we use a vendor version of the CLI


public function initialize() : void
{
$this->reader = new CachedReader(new AnnotationReader(), new ArrayCache());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should probably use a void cache in this sort of test. Also clashes with CachedReader removal - maybe take the chance and get rid of it overall?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

#249 😛

@Ocramius Ocramius self-assigned this Feb 26, 2019
@Ocramius Ocramius added this to the 1.7.0 milestone Feb 26, 2019
Copy link
Copy Markdown
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

🚢

Note that this should hit a 1.x release before we push for 2.x, as otherwise we don't know if we're making things better or worse.

@Ocramius Ocramius merged commit 95c17ef into doctrine:master Feb 26, 2019
@Majkl578 Majkl578 deleted the phpbench branch February 26, 2019 15:26
@Ocramius
Copy link
Copy Markdown
Member

As per discussion with @alcaeus and @Majkl578, master is already 2.0.0, so this is going there.

@Ocramius Ocramius modified the milestones: 1.7.0, v2.0.0 Feb 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants