Skip to content

Optimize performance#103

Closed
samdark wants to merge 4 commits intomasterfrom
performance
Closed

Optimize performance#103
samdark wants to merge 4 commits intomasterfrom
performance

Conversation

@samdark
Copy link
Member

@samdark samdark commented Feb 9, 2025

Before:

\Yiisoft\Definitions\Benchmark\DefinitionStorageBench

    benchSetAndGet..........................R1 I6 - Mo0.929μs (±1.08%)
    benchHas................................R1 I6 - Mo0.727μs (±3.03%)
    benchSetMultipleDefinitions.............R3 I9 - Mo1.660μs (±0.92%)
    benchGetBuildStack......................R2 I6 - Mo1.971μs (±2.10%)

After:

\Yiisoft\Definitions\Benchmark\DefinitionStorageBench

    benchSetAndGet..........................R1 I0 - Mo0.770μs (±0.70%)
    benchHas................................R1 I8 - Mo0.597μs (±2.40%)
    benchSetMultipleDefinitions.............R2 I7 - Mo1.239μs (±1.62%)
    benchGetBuildStack......................R2 I9 - Mo1.847μs (±3.34%)

@samdark samdark requested a review from a team February 9, 2025 22:26
}
}

if (!$isUnionTypeResolvable) {
Copy link
Member

Choose a reason for hiding this comment

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

????

Copy link
Member Author

@samdark samdark Feb 11, 2025

Choose a reason for hiding this comment

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

Yeah. "Optimization". Tests pass, so "why not" :) I'll revert it.

Copy link
Member

@vjik vjik left a comment

Choose a reason for hiding this comment

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

Seems, all changes break logic.

Yii Definitions package doesn't contain enough tests. Any changes should be tested in Yii DI also.

@samdark
Copy link
Member Author

samdark commented Feb 12, 2025

I think it doesn't worth trying to fix this code.

@samdark samdark closed this Feb 12, 2025
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.

4 participants