Skip to content

Deprecate ticks#6967

Closed
nikic wants to merge 1 commit into
php:masterfrom
nikic:deprecate-ticks
Closed

Deprecate ticks#6967
nikic wants to merge 1 commit into
php:masterfrom
nikic:deprecate-ticks

Conversation

@nikic

@nikic nikic commented May 10, 2021

Copy link
Copy Markdown
Member

@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
Copy Markdown
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

nikic commented May 12, 2021

Copy link
Copy Markdown
Member Author

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

mvorisek commented May 12, 2021

Copy link
Copy Markdown
Contributor

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
Copy Markdown

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

mvorisek commented May 14, 2021

Copy link
Copy Markdown
Contributor

Memory tracing is easily possible with xdebug...

@matthieu88160

Copy link
Copy Markdown

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

@jeichorn

Copy link
Copy Markdown

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

derickr commented Jun 10, 2021

Copy link
Copy Markdown
Member

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

stollr commented Jun 11, 2021

Copy link
Copy Markdown

@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
Copy Markdown

@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

nikic commented Jul 2, 2021

Copy link
Copy Markdown
Member Author

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