Skip to content

feat: Slight performance improvements#29

Open
kissifrot wants to merge 1 commit intocloudinary:mainfrom
kissifrot:chore/slight-perfs-improvments
Open

feat: Slight performance improvements#29
kissifrot wants to merge 1 commit intocloudinary:mainfrom
kissifrot:chore/slight-perfs-improvments

Conversation

@kissifrot
Copy link
Copy Markdown

Brief Summary of Changes

Several targeted performance improvements on the serialization hot path (qualifier key/name resolution, multi-value qualifier rendering), plus a regression test and expanded coverage for isJsonString().
No external behavior change.


What does this PR address?

  • GitHub issue (Add reference - #XX)
  • Refactoring
  • New feature
  • Bug fix
  • Adds more tests

Are tests included?

  • Yes
  • No

Details

  • QualifierMultiValue::__toString() — mutation bug fix (biggest gain) EffectName argument keys were appended directly to $this->argumentOrder on every call. Any code path serializing the same qualifier more than once (URL generation loops, logging, test assertions) caused $argumentOrder to grow unboundedly, degrading sort performance on subsequent calls.
    Fix: operate on a local copy of $argumentOrder inside __toString().

  • BaseQualifier::getKey() — per-class static cache
    getKey() invoked getName() + StringUtils::toAcronym() on every call.
    The result depends solely on the class name and is immutable per process.
    Added a private static array $keyCache keyed by static::class (Late Static Binding) so each concrete subclass pays the derivation cost once.

  • BaseComponent::getName() — per-class static cache
    Same rationale: getName() called ClassUtils::getBaseName() + StringUtils::camelCaseToSnakeCase() on every invocation. Added private static array $nameCache with the same LSB pattern.

  • JsonUtils::isJsonString() — fast path + PHP 8.3+

    • Added a first-character guard ({ or [) before any parsing, preserving the original semantics of accepting only JSON objects and arrays.
    • PHP 8.3+: delegates to json_validate(), which validates JSON syntax without building a decoded value in memory.

Benchmark

Scenario Before After Gain
Qualifier serialized ×10 (mutation fix) 7 092 ms 395 ms ×18
Full pipeline — single step 527 ms 243 ms ×2.2
Full pipeline — multi step 1 339 ms 689 ms ×1.9
Full pipeline — complex (overlay) 2 468 ms 1 707 ms ×1.4
isJsonString() — large payload 84 ms 68 ms ×1.2

Reviewer, please note:

  • The $nameCache / $keyCache arrays live on the respective base classes and are keyed by static::class, so each concrete subclass gets its own entry while the storage stays on the base class. The caches are per-process (transparently reset on each PHP-FPM request).
  • The QualifierMultiValue change is a pure bug fix — the old code produced incorrect sort order after the first __toString() call on any instance whose EffectName arguments were not pre-sorted. A regression test is included in EffectTest::testEffectQualifierToStringIsIdempotent().
  • isJsonString() behaviour is unchanged with one narrow exception: strings with leading whitespace before {/[ (e.g. " {}") now return false.
    This was not a supported use-case in practice (configs are always programmatically generated without leading whitespace), but indicate me if I need to alter it 👍

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I ran the full test suite before pushing the changes and all the tests pass.

@kissifrot
Copy link
Copy Markdown
Author

@const-cloudinary if you happen to pass by 😇 🙏

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.

1 participant