Skip to content

Deprecate ticks #6967

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

Closed
wants to merge 1 commit into from
Closed

Deprecate ticks #6967

wants to merge 1 commit into from

Conversation

nikic
Copy link
Member

@nikic nikic commented May 10, 2021

@nikic nikic added the RFC label May 10, 2021
@nikic nikic force-pushed the deprecate-ticks branch from f0f18be to 3dfd732 Compare May 10, 2021 13:53
@mvorisek
Copy link
Contributor

register_tick_function can be usefull for debugging atk4/core#320 to call gc_collect_cycles after every operation, declare(ticks=N) is however not needed for this usecase

if register_tick_function is removed, can simillar behaviour (to call gc_collect_cycles after every operation) be registered with any other function or with xdebug?

@nikic
Copy link
Member Author

nikic commented May 12, 2021

register_tick_function can be usefull for debugging atk4/core#320 to call gc_collect_cycles after every operation, declare(ticks=N) is however not needed for this usecase

I don't understand what you mean here. Without declare(ticks=N), register_tick_function() will not do anything.

@mvorisek
Copy link
Contributor

mvorisek commented May 12, 2021

I don't understand what you mean here. Without declare(ticks=N), register_tick_function() will not do anything.

declare(ticks=N) can be generated to every php file easily...

UPDATE: gc_collect_cycles can be called in a getter before the result is returned, yes, the ticks functionality is probably useless and I am also for the removal 👍

@matthieu88160
Copy link

Hi all,

I technically agree with the deprecation of the ticks function but have to point interest for it.

Our application currently uses ticks to trace memory and time consumption of parts of code in addition to a custom stream wrapper to apply the declare(ticks=N) in every file. Remove the tick function will lead us to refactor the whole application or to improve the wrapper in a way that injects statements in code.

This is for sure not the subject of this RFC, but I would suggest adding a way to trigger an event on blocks in/out before removing tick's support.

@mvorisek
Copy link
Contributor

mvorisek commented May 14, 2021

Memory tracing is easily possible with xdebug...

@matthieu88160
Copy link

Sure, but with a major impact on performance, we are using ticks to mitigate this drawback in production.

@jeichorn
Copy link

A wordpress plugin uses ticks + stream wrappers for profiling, it has a decent # of installs, and is the only profiling option available at many shared hosts
https://wordpress.org/plugins/p3-profiler/
https://github.com/wpmudev/wp-p3-profiler

xdebug etc aren't reasonable alternatives since they aren't 100% available while ticks based approaches work everywhere.

@derickr
Copy link
Member

derickr commented Jun 10, 2021

As these "profilers" use stream wrappers to rewrite the files to add the declare ticks, they can also rewrite the code to add calls to their function that is now called from within their registered tick function. Sure, it's not as easy, but it's possible.

@stollr
Copy link

stollr commented Jun 11, 2021

@matthieu88160 I don't know if that would help you, but did you have a look at blackfire.io?

Profiling is done on demand. The only request showing overhead is the one being profiled, only for the profiling session. No other session or request is impacted. You can safely use Blackfire on production servers.

@derickr Your statement is only true for the code you own, but not for 3rd party librarys ;)

@matthieu88160
Copy link

@matthieu88160 I don't know if that would help you, but did you have a look at blackfire.io?

Thank @Naitsirch, we already have a Blackfire system installed for our development team and internal use but, it does not fit our needs for some client installation where we face restrictions to install this kind of third-party services.

Out of our specific constraints, Blackfire is a good solution if you can install it.

@nikic
Copy link
Member Author

nikic commented Jul 2, 2021

Based on the comments here, I've decided to withdraw this RFC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants