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

Introduce a simple cache and build a new ReferenceContext only when needed #158

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

Conversation

marcelthole
Copy link
Contributor

hey, maybe this could work and improve the perfomance for the use case in #156.

Here is a simple repo script:

<?php

require 'vendor/autoload.php';

$start = hrtime(true);
$openapi = \cebe\openapi\Reader::readFromYamlFile(__DIR__ . '/index.yaml');
$end = hrtime(true);

var_dump(number_format(($end - $start) / 1e6,4, ',', ' ') . ' ms');

and this are the before and after results
(left after the change, right before)
grafik

@marcelthole marcelthole force-pushed the resolve-perfomance-156 branch from 26a5d92 to 9edcbd2 Compare March 11, 2022 17:25
@@ -29,6 +29,9 @@
*/
class Reference implements SpecObjectInterface, DocumentContextInterface
{
/** @var array<string, mixed> */
private static $relativeReferencesCache = [];
Copy link
Owner

Choose a reason for hiding this comment

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

this could probably cause issues when the library is used to read multiple different OpenAPI descriptions in the same process. Ideally the cache should be bound to the context like we have it with ReferenceContextCache.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah you are right. I used the ReferenceContextCache.
But this reduced the perfomance of the first variant. The first version only used 1.3 Million times this method, the current version 4.3 Million times. But it's still better than the original version with 9.8 Million function calls

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.

2 participants