Skip to content
This repository has been archived by the owner on Dec 16, 2019. It is now read-only.

Feasibility of thread-local non-static properties on Threaded objects #873

Open
dktapps opened this issue May 29, 2018 · 8 comments
Open
Assignees

Comments

@dktapps
Copy link
Contributor

dktapps commented May 29, 2018

Summary

It may often be reasonable to want to store some data on a Threaded object that does not get serialized, for later accessing only on the parent thread - for example in a worker task based architecture where the task has a generic onCompletion() function which may want to operate on objects on the main thread.

Currently pthreads doesn't provide any reasonable way to do this (static properties are unsuitable because different tasks of the same classes may want to store different data).

I am wondering on the feasibility of implementing some kind of @thread_local annotation which could be applied to Threaded member fields to make them suitable for storing objects like this, or possibly a ThreadLocal interface if this isn't possible (although I think it would be better to be able to apply this to non-objects too, for example resources).

Currently it is necessary to separately store any objects that a worker task may want to operate on after completion, because they will become serialized if assigned to the task object itself.

Example Code

This is a bad example, but I think it demonstrates the point well enough to work with.

<?php

declare(strict_types=1);

require_once 'vendor/autoload.php';

interface Completable{
	public function onCompletion() : void;
}

class MyObject{
	/** @var string */
	private $content;

	public function setContent(string $contents) : void{
		$this->content = $contents;
	}
}

$worker = new \Worker();
$worker->start();

$object = new MyObject();

$worker->stack(new class($object) extends \Threaded implements Completable{

	/**
	 * @thread_local
	 * We're only storing this for use in onCompletion(), we don't want it to be serialized
	 *
	 * @var MyObject
	 */
	private $object;

	private $result;

	public function __construct(MyObject $object){
		$this->object = $object;
	}

	public function run() : void{
		//do some work
		$this->result = random_bytes(32);
	}

	public function onCompletion() : void{
		$this->object->setContent($this->result);
	}
});

//go away and do some other things
sleep(1);

//come back and collect tasks
$worker->collect(function(Collectable $collectable) : void{
	if($collectable instanceof Completable){
		$collectable->onCompletion();
	}
});
@tpunt
Copy link
Collaborator

tpunt commented May 30, 2018

I think this is an interesting idea, and that TLS should be explored more. With pht, I've made everything TLS-only, but still provided the ability to communicate between threads with specific data structures. With pthreads, we could look into having a balance by giving the ability to share properties between threads (and therefore incurring serialisation overhead/limitations), as well as having properties that are TLS only (by doing something similar to what you suggested). Or perhaps each Thread could have a special $tls property that could be for thread-local data only. This should be relatively simple to implement, and provide much more flexibility for threaded objects that don't need to pass certain data between contexts.

Regarding your idea, I'm unsure about the implementation (including the performance considerations of it). I don't really have time to look into this at the moment, either. It would be nice to hear some further ideas around this area, though, as well as perhaps seeing a few proof-of-concept patches to play around with.

@dktapps
Copy link
Contributor Author

dktapps commented May 30, 2018

I haven't thought too much about how this would actually be implemented just yet. Language level support would be a godsend, but in its absence I can only come up with strange hacks and workarounds. I'll have to think about this more. I also considered magic properties, but those could result in astonishing behaviour for other developers.

At the moment I resorted to static properties containing SplObjectStorages indexed by Threaded, but this isn't ideal as can be imagined.

@sirsnyder
Copy link
Collaborator

sirsnyder commented Oct 2, 2018

Just pushed a first draft on a07d15a. Supported annotations for properties are @thread_local and @threadLocal. Your feedback is highly appreciated!

class AnnotatedThreadLocals extends Threaded {
        /**
         * @thread_local
         * @threadLocal
         * @var string
         */
        public $myThreadLocalProperty;

        /**
         * @var string
         */
        public $myThreadedProperty;
    }

@dktapps
Copy link
Contributor Author

dktapps commented Oct 2, 2018

Holy crap that's awesome. Nice work @sirsnyder, can't wait to test this.

@dktapps
Copy link
Contributor Author

dktapps commented Oct 2, 2018

@sirsnyder zend_std_has_property and zend_std_unset_property are static functions, not exported in any header. The extension won't link because of this.

@dktapps
Copy link
Contributor Author

dktapps commented Oct 2, 2018

[With some fixes for compilation] It also crashes when accessing class properties which are not public because zend_get_property_info() returns ZEND_WRONG_PROPERTY_INFO. This test case reproduces it:

<?php

class Test extends Thread {

	public function run(){
		var_dump($this);
	}
	
	protected $string = "hello world";
	protected $array  = array(1, 2, 3);
	private $pstring  = "world hello";
	private $parray   = array(3, 2, 1);
	protected static $nocopy = true;
}

$test =new Test();
$test->string = strrev($test->string);
$test->start();
$test->join();
?>

This code is already invalid. I can only assume the reason this test case being broken was missed is because of a combination of behavioural changes in v3 and #903 .

@dktapps
Copy link
Contributor Author

dktapps commented Oct 2, 2018

You might also consider moving these checks to pthreads_store_read() and pthreads_store_write() instead so that they work for array-accessing predefined properties.

@sirsnyder
Copy link
Collaborator

sirsnyder commented Oct 9, 2018

Thanks for first feedback! Ultimately I would not like to touch Threaded or Volatile for these enhancements. As far as I remember, Joe developed Threaded with performance/speed in mind. After my snivelling of BC, he introduced Volatile. Therefore I would tend to add a new class, a third one besides Threaded or Volatile. Features of this new class could be property visibility, proper __get/__set handling, arrayaccess and @thread_local annotations. Performance is subordinate. But I can not think of a good name, I am still looking for it. Maybe Concurrent?

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

No branches or pull requests

3 participants