pthreads icon indicating copy to clipboard operation
pthreads copied to clipboard

WIP: New Concurrent class

Open sirsnyder opened this issue 7 years ago • 2 comments

Introduction of Concurrent class. This new mutable class features property visibility, a thread-local annotation(@thread_local), ArrayAccess callback support and proper __get/__set/__isset/__unset handling.

Additionally this PR fixes #861, however with a BC break. Copying of static properties is confined to only default values and not assigned values.

sirsnyder avatar Oct 18 '18 01:10 sirsnyder

I'm not sure I understand the deal with thread-locals. Why is it necessary to confine them to a new class? What if I want thread locals without the performance hit involved with mutability? The additional runtime checks appear to be very minor, so I don't see why they should be relegated to a separate class.

I think it would be better if the statics fix could be handled separately from the Concurrent implementation so that can be integrated independently.

dktapps avatar Oct 20 '18 08:10 dktapps

I'm not so sure I like introducing a new class for this. Is the performance overhead that much, since this is only done at copy time? I get that it introduces a little extra overhead, but given that it has the potential to save on some serialisation overhead (which is forcibly incurred on every instance property currently), the potential speed improvements of TLS could negate this. Maybe some benchmarks would help to see this more clearly.

Also, the name "Concurrent" isn't particularly good, since it is pretty generic. It could refer to any concurrency primitive (thread, process, actor, communicating sequential process, green thread, etc).

I'm glad to see an implementation of this, though :)

tpunt avatar Oct 20 '18 14:10 tpunt