With that in mind I'd think about - for the sake of interface segregation - removing thenow function completely from theTimeInterface. The different implementations can then still implement the different interfaces in one class. And should more than one interface be required in a method we can by now require an implementation via a UnionType.

Hi Andreas,

Why would we want to remove thenow method? The whole point of this component is actually to have such method.
Could you please develop?

I doubt that PSR-20 is going to be ready by the release of Symfony 6.2.

🤷 hurry up PHP-FIG :)

for the sake of interface segregation - removing the now function completely from the TimeInterface (and perhaps also moving the sleep function into a separate interface - we've discussed that during the PSR-20 discussions IIRC).

I'd better not as I explained in#46715 (comment)
If we end up being incompatible with PSR-20, we'll deal with it with a deprecation layer.
But looking at the state of PSR-20, it looks unlikely that we'll have to do that.

Thanks for the review btw@heiglandreas !

With that in mind I'd think about - for the sake of interface segregation - removing thenow function completely from theTimeInterface. The different implementations can then still implement the different interfaces in one class. And should more than one interface be required in a method we can by now require an implementation via a UnionType.

Hi Andreas,

Why would we want to remove thenow method? The whole point of this component is actually to have such method. Could you please develop?

Thenow-method would be already defined in aClockInterface - likepsr/clock orstella-maris/clock, so defining it here again in aTimeInterface is not really necessary. The actualImplementation then would implementTimeInterface, ClockInterface, SleepInterface.

So the component would either require an externalClockInterface or declare a spearateClockInterface in parallel to the currentTimeInterface and a currently not existingSleepInterface.

The now-method would be already defined in a ClockInterface - like psr/clock or stella-maris/clock, so defining it here again in a TimeInterface is not really necessary. The actual Implementation then would implement TimeInterface, ClockInterface, SleepInterface.

I want the component to be standalone and complete on its own. When PSR-20 will be ready, we might extend it, but I wouldn't make a stable PSR-20 a requirement to this component.

So the component would either require an external ClockInterface or declare a spearate ClockInterface in parallel to the current TimeInterface and a currently not existing SleepInterface.

Replying in the main thread to help readers follow the discussion: see#46715 (comment) about this topic.

Thanks for this proposal!

In the description we see this:

timeInt(): float returns the current timestamp;

I guess it's a typo thattimeInt() returns afloat instead ofint.

If the naming of the class/methods is up for debate, here's a proposal for your consideration:

// Beforeinterface TimeInterface{publicfunctionnow():\DateTimeImmutable;publicfunctionsleep(float|int$seconds):void;publicfunctiontimeInt():int;publicfunctiontimeFloat():float;publicfunctiontimeArray():array;}// Afterinterface ClockInterface{publicfunctionnow():\DateTimeImmutable;publicfunctionsleep(float|int$seconds):void;publicfunctiontimestamp():float;}

I'd removetimeInt(),timeFloat() andtimeArray() ... but you said that they are important for performance reasons. So, why not merging all of them into a single method calledtimestamp() which returns a timestamp with microseconds precision. It'd be equivalent totimeFloat() ... and thetimeInt() value can be obtained as(int) $clock->timestamp(). About thetimeArray() method, I don't understand well its need.

Thanks.

Brain activity at night made me realize that we could remove thetimeInt() andtimeArray() variants and keep onlytime(), providedtime() returns a string. As a reminder, the concern this addresses is loss-less time measurements on 32-bit archs. This is the same concern that leads to havingmicrotime(false) also return a string.

Then I did my homework and tried to verify my own claim about performance, which is the justification I've provided for thesetime*() methods.

Comparingnew \DateTimeImmutable('now', $tz) tomicrotime(true), the latter is almost 5x faster.
But when comparingnew \DateTimeImmutable('now', $tz) to(string) microtime(true), the latter is only 1.1x faster!

This difference is not significant enough to warrant my claim. I'm thus removing alltime*() methods from my proposal. Performance-critical time measurements might continue to useClockMock since we can't abstract time measurements without adding overhead to it. Note that using the component for not-so-critical performance measurements is still valid, because we're talking about quite fast measurements.

Since this means the interface now contains onlynow() andsleep(), I've renamed it toSleepableClockInterface.

I've also implemented the offset logic@derrabus suggested forHrClock, which is now renamedMonotonicClock (because it's not "high-resolution" anymore, see notes about the overhead above.)

I updated the PR and the description above with all those changes.

I think this addresses most if not all concerns raised so far.

The last thing that remains is the lack of aClockInterface, and that's on the PHP-FIG 🙏

I'm looking forward to the newest PSR-20 and this component! At Sylius, we've recently introduced theCalendar component, but we will surely try to migrate to the use yours. Our use case mainly decouples implementation fromnew in the code and provides ease-of dates testing - we need to freeze the clock and set its time for some given moment. If you are open to SettableClock implementation and interface, I would love to try it.

we need to freeze the clock and set its time for some given moment. If you are open to SettableClock implementation and interface, I would love to try it.

Sure we are if this proves needed :)

After readingphp-fig/fig-standards#1257 I'm wondering if it wouldn't make sense to addwithTimeZone()?

That'd allow registering only one clock service and give a contract to users to stick to a TZ.

Loosely related, I'm wondering if we shouldn't swap our approach to naming and addClockInterface to the component. That'd be the only interface provided by the component.

The benefit of this approach is that it would make us less dependant on the outcome and pace of the FIG.

My preference goes to doing both changes I propose here: one capable ClockInterface in the component and one more specific in PSR20 when it'll be released.

PR updated with the approach proposed in my previous comment:
The component now provides aClockInterface with 3 methods:now(),sleep() andwithTimeZone().

I also registered aclock service into FrameworkBundle and a corresponding autowiring alias for ClockInterface.
Since we now havewithTimeZone(), there is no need to provide several clock services (an UTC one and a default-TZ one.)

When PSR20 will be out (and if it remains compatible with the component), ppl will have the choice to use either the generic and narrow abstraction from the FIG - or the more capable one from the component when in need of extra features.

/cc @symfony/mergers even if you already voted, please vote again if you approve these changes.

Thank you@nicolas-grekas.