Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Scheduler] Improve triggers performance when possible#49817

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

fabpot
Copy link
Member

@fabpotfabpot commentedMar 26, 2023•
edited
Loading

QA
Branch?6.3
Bug fix?yes-ish
New feature?yes
Deprecations?no
Ticketsn/a
LicenseMIT
Doc PRn/a

Using\DatePeriod for the default trigger is great as it takes care of all date/time idiosyncrasies.
But for high frequencies, that does not work well and performance becomes an issue. So, this PR solves this issue by always using the fast algorithm when the frequency is expressed in seconds (int) or an ISO period (likePT2S) or when created from a string that uses a "simple" expression (like2 seconds).

/cc@upyx

@upyx
Copy link
Contributor

Why do you think the original version was bad? TheDatePeriod class doesn't fit the requirements of the triggers: it provides a forward only iterator. ThegetNextRunDate method must return a valid value for any argumentin any order. It's just a different task.

Now, there are two algos and "magic" with reflections...

The goal of constructors is initialization of objects to a valid state. It's not a big deal, but having logic in a constructor except assertions of arguments is bad practice. Now it has too much "magic".

TheTriggerInterface allows you to make any composition. Triggers (originally) are not intended to be extended.

IMO it was better.

@fabpotfabpotforce-pushed thescheduler-trigger-perf-improvments branch from47b35b8 tofe266a5CompareMay 21, 2023 14:25
@fabpotfabpot requested a review fromkbond as acode ownerMay 21, 2023 14:25
@fabpotfabpotforce-pushed thescheduler-trigger-perf-improvments branch 2 times, most recently from537e357 to55933a2CompareMay 21, 2023 14:36
@fabpotfabpotforce-pushed thescheduler-trigger-perf-improvments branch 5 times, most recently from6e48389 toc494148CompareMay 21, 2023 19:34
@fabpotfabpotforce-pushed thescheduler-trigger-perf-improvments branch fromc494148 tof70a577CompareMay 21, 2023 19:39
Copy link
Member

@chalasrchalasr left a comment

Choose a reason for hiding this comment

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

cosmetic

@fabpotfabpotforce-pushed thescheduler-trigger-perf-improvments branch fromf70a577 toc80e9adCompareMay 21, 2023 19:45
@fabpotfabpotforce-pushed thescheduler-trigger-perf-improvments branch fromc80e9ad toa53187fCompareMay 21, 2023 19:47
@fabpotfabpotforce-pushed thescheduler-trigger-perf-improvments branch 2 times, most recently from1fe7222 toca3c30cCompareMay 22, 2023 08:26
@nicolas-grekas
Copy link
Member

Thank you@fabpot.

@nicolas-grekasnicolas-grekas merged commit9b0d811 intosymfony:6.3May 22, 2023
@fabpotfabpot mentioned this pull requestMay 22, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@upyxupyxupyx left review comments

@chalasrchalasrchalasr requested changes

@kbondkbondkbond approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
6.3
Development

Successfully merging this pull request may close these issues.

6 participants
@fabpot@upyx@nicolas-grekas@kbond@chalasr@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp