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

Improving the documentation #926

Open
joeyhub opened this issue Feb 13, 2019 · 9 comments
Open

Improving the documentation #926

joeyhub opened this issue Feb 13, 2019 · 9 comments

Comments

@joeyhub
Copy link

joeyhub commented Feb 13, 2019

I don't find it clear at all how use this extension especially in relation to data exchange.

Honestly' I can barely say I've seen anyone quite work out how to use this or make good use of it. The comments in the PHP manual are particularly revealing to me of this.

Can you confirm:

That the thread runs in a new PHP instance with entirely it's own data barring extensions that aren't thread safe, so you must for example run the autoloader again and possible also rerun bootstrap in certain scenarios? As in it's basically the same as if the PHP engine went from being a static class to a normal class? I see class definitions copied from parent to child so something's shared/copied.

Are class properties deeply immutable?

Do volatile properties allow data access between parent and child? Is it safe?

What's the shift and pop for?

What's the point of extend? It looks like a horrific hack.

Why is it bad for web? Performance concerns aren't relevant, that's usage and implementation dependent. Is there a safety issue? Do they clean up poorly or clash with thread management in the web server?

What are the rules for data transfer such as automatically syncronized, copy/clone, valid types, wrapping and unwrapping, etc?

It appears I can pass the parent thread to it's children and then it's children can notify it. Is this officially supported (threads passing thread objects to each other at will)?

Is it really not possible to get a thread object for the initial thread?

The example for synchronized makes no sense without a loop.

Does syncronized run under the caller thread? Wait only works in syncronized.

What's a context?

In some cases you may want to synchronise output (var dump for example).

Is it possible for documentation to explain differences between method calls from inside or ooutside (what happens if $this->join())?

@dktapps
Copy link
Contributor

dktapps commented Feb 13, 2019

#709

@ghost
Copy link

ghost commented Feb 13, 2019

Why is it bad for web? Performance concerns aren't relevant, that's usage and implementation dependent. Is there a safety issue? Do they clean up poorly or clash with thread management in the web server?

A quote from today's blog article by krakjoe:

Threading at the frontend of a website, on the face of it, doesn't make sense: If you have a web request that creates a "reasonable" number of threads, let's say 8, and 1000 clients come along at once, you are asking your hardware to execute 8*1000 threads not including the threads or processes that done the creation, this is unreasonable and cannot possibly scale (with a 1:1 threading model, which is what pthreads has). That said, other languages do manage to integrate parallelism into the web response, but it's tricky, and takes a lot of thought and expertise. I've never suggested that you should build software in this way, and would never suggest it, eventually I prohibited the use of pthreads in anything but the CLI in an attempt to force users to be reasonable.

https://blog.krakjoe.ninja/2019/02/parallel-php-next-chapter.html

@joeyhub
Copy link
Author

joeyhub commented Feb 13, 2019

I appreciate that but that's also a performance related argument only applicable for certain cases.

Notice for example it's conditional, it has an if, which I specifically asked not for. I don't want an oh well in the worst case argument... managing load and implementation is on me, the user, not the writer of the library. They're not responsible if I overwhelm hardware with it and the same argument can apply to the library even as a CLI application. It's an argument not to use threads at all.

What I want is a meaningful reason that's more absolute such as either conflicts with the thread management with the software it's integrated into or the opposite, that software not allowing the management of threads, for example cleaning up.

If the reason is, but if suddenly a thousand requests (even in scenarios where you might control the request rate) then that's not a real reason, it's an excuse or an opinion. Saying that his comment basically concedes, can't be bothered which to be fair gets to the bottom of it. In the readme it's not so clear. It's a reason enough that it's a hassle to support web but annoying when it's put on the user in the docs.

@joeyhub
Copy link
Author

joeyhub commented Feb 13, 2019

Manual again:

public Threaded::pop ( void ) : bool
The above example will output:
int(9)

@dktapps
Copy link
Contributor

dktapps commented Feb 13, 2019

The documentation is out of date in many places. You might be better looking at the stub.php in the examples/ directory.

@joeyhub
Copy link
Author

joeyhub commented Feb 13, 2019

I'll try to share what I learn as I'm getting things to work but I think it really is an unappreciated problem. It makes the lib hard to design as well going forward since the doc is pretty much also a spec. Things like using Volatile as just a wrapper for something being mutable isn't at all clear all intuitive and looks like an obvious design fudge to me if that we're documented more clearly.

For example if you're passing it just to basically to manage property access between threads there's a bunch of extra baggage like that such as the run function.

Some less terse implementation or at least up to date documentation fills in those voids and can present design fudges early.

On a similar note people jumping to use annotation. I might recommend instead a meta data or basically config in PHP. You can always then map annotation to that and let user space worry about it.

All the bean is can just be something like getClassConf which might return a map of methods and functions with additional modifiers PHP doesn't support as keywords.

I was also going to ask if the accessors still enforce access but also add validation that the thread is running (state checking) or if they replace it but there's an issue open on that already.

@dktapps
Copy link
Contributor

dktapps commented Feb 13, 2019

Oh absolutely. If I may say so the API as it stands is really not very coherent, and I say this after 2+ years of working with a project that uses it. It involves a lot of feeling around in the dark I must say.

@ghost
Copy link

ghost commented Feb 13, 2019

On a similar note people jumping to use annotation. I might recommend instead a meta data or basically config in PHP. You can always then map annotation to that and let user space worry about it.

All the bean is can just be something like getClassConf which might return a map of methods and functions with additional modifiers PHP doesn't support as keywords.

I'm not sure what you mean by that. Annotations are mostly used for documentational purposes. But sure, there are Doctrine and similar projects which use annotations for more than that - they inject functionality on top of that, and to be honest, I don't like that. It's error prone and open to mistakes, while a PHP OOP API design would lead to less errors in my opinion.

@joeyhub
Copy link
Author

joeyhub commented Feb 13, 2019

Annotations can be used to extend the language, basically adding new keywords. The obvious case is how people use it for typehinting properties until PHP adds official support for that.

$name = /** @translate */'String';

/** @modifiers(immutable) */private $var;

It's sort of documentation but can actually be more than that, it can be put into use. Obvious example here is that threaded classes are ambiguous as now it's either the thread itself accessing or another thread accessing class members. People have pointed out there's a need to know if for example properties are thread local or even mark them for optimisation. We also have hijacking existing keywords. Like I think protected in place of Java's explicit syncronize keyword.

Annotation could be a stop gap to improve documentation but for actionable things there should be class metadata such as:

interface ThreadMeta {
    public function getConfig() {
        return ['properties' => ['a' => 'local', 'b' => 'final'], 'methods' => ['getThing' => 'local,synchronized']];
    }
}

With doc saying what the defaults are (class can be more fleshed out but one function to rule them all works best for quick example).

You would pretty much need that anyway for annotation to inject into. The concern is if the C under the hood starts trying to read annotation rather than giving the user the choice. Have the meta data mechanism where it's all just straight PHP then people can overlay that with what they want including implementing some custom annotation handler for whatever framework they have or any other kind of config provider. Basically if people want annotation, they can make that, if they don't then they don't have to.

I do think though just playing around with pseudo annotation a little and sort of inventing a language to make everything explicit would be an interesting thought exercise to drive out design concerns in places. I guess initially I dumbed it down a bit but basically some pseudo code and annotation is useful for playing around and could help improve D&D but I also wouldn't favour direct support for it, would be more strange things under the hood.

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

No branches or pull requests

2 participants