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

[BUGFIX] Enhance alias resolving and reduce file system invocation [v3] #31

Merged
merged 3 commits into from
May 5, 2019

Conversation

ohader
Copy link
Member

@ohader ohader commented Apr 27, 2019

  • Consider direct alias invocation without being registered
    Invoking an aliased phar like phar:///path/to/phar.phar/autoload.php
    would allow to extract and register a potential alias name of the base
    name at /path/to/phar.phar. Using alias names directly inside phar
    archive does provide any possibility to map its full path to an alias
    (e.g. directly invoking phar//alias.phar/autoload.php would fail).
    This change aims to resolve the original base name by walking the
    current stack trace backwards. In terms of performance this is not
    ideal, but it's the only chance to retrieve the required information.
    That's also why the next section addresses performance.

  • Enhance performance by reducing file system invocation
    Interceptors have to resolve the base file name from some given
    Phar invocation request. Internally the given path is traversed until
    a valid file is found in the file system and considered as the base
    name of the Phar archive.
    This change reduces superfluous calls to is_file when splitting
    the path in order to resolve the actual base name.

Resolves: #21
Resolves: #23

These tests just reflect the status quo without changing anything.
* Consider direct alias invocation without being registered
  Invoking an aliased phar like `phar:///path/to/phar.phar/autoload.php`
  would allow to extract and register a potential alias name of the base
  name at `/path/to/phar.phar`. Using alias names directly inside phar
  archive does provide any possibility to map its full path to an alias
  (e.g. directly invoking `phar//alias.phar/autoload.php` would fail).
  This change aims to resolve the original base name by walking the
  current stack trace backwards. In terms of performance this is not
  ideal, but it's the only chance to retrieve the required information.
  That's also why the next section addresses performance.

* Enhance performance by reducing file system invocation
  Interceptors have to resolve the base file name from some given
  Phar invocation request. Internally the given path is traversed until
  a valid file is found in the file system and considered as the base
  name of the Phar archive.
  This change reduces superfluous calls to `is_file` when splitting
  the path in order to resolve the actual base name.

Resolves: #21
Resolves: #23
* PharInvocation has to be confirmed now
* avoid Phar archive parsing in Phar\Reader
* extend interface Collectable::has(PharInvocation)
* enhance overall performance for PharMetaDataInterceptor
@ohader ohader changed the title [BUGFIX] Enhance alias resolving and reduce file system invocation [BUGFIX] Enhance alias resolving and reduce file system invocation [v3] Apr 27, 2019
@ohader ohader merged commit fff6f5e into master May 5, 2019
@ohader ohader deleted the bugfix/alias-performance/master branch May 6, 2019 09:42
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.

Performance down between 0469d9f and b7a21f0 Breaking included AWS Phar
1 participant