Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Naming concerns #4

Open
dminor opened this issue Jul 25, 2024 · 2 comments
Open

Naming concerns #4

dminor opened this issue Jul 25, 2024 · 2 comments

Comments

@dminor
Copy link

dminor commented Jul 25, 2024

We have some concerns about the use of Governor and Semaphore when we reviewed this proposal.

No one was a big fan of the use of Governor for this, the car metaphor didn't work for us, and we thought that the naming didn't give a good indication of the functionality.

There were also concerns about the use of Semaphore. I would think that the expectation about anything named semaphore is that it would be used to coordinate between multiple threads, but the concurrency control being investigated here actual seems to be single-threaded and between async operations, although to be honest, I'm not entirely sure. It would be great to see this stated explicitly in the explainer.

@bakkot
Copy link

bakkot commented Jul 25, 2024

I would think that the expectation about anything named semaphore is that it would be used to coordinate between multiple threads, but the concurrency control being investigated here actual seems to be single-threaded and between async operations, although to be honest, I'm not entirely sure. It would be great to see this stated explicitly in the explainer.

Coordination between threads is certainly the textbook example, but controlling access to resources in a concurrent-but-not-parallel system is basically the same problem with the same shape of solution, and people still call it a semaphore in that case. See e.g. various npm packages.

Also per the readme the idea is that these would be shareable across threads (which would require some host integration, and underlying use of atomics), so it would be useful for the classical parallel case as well.

@dminor
Copy link
Author

dminor commented Jul 26, 2024

I would think that the expectation about anything named semaphore is that it would be used to coordinate between multiple threads, but the concurrency control being investigated here actual seems to be single-threaded and between async operations, although to be honest, I'm not entirely sure. It would be great to see this stated explicitly in the explainer.

Coordination between threads is certainly the textbook example, but controlling access to resources in a concurrent-but-not-parallel system is basically the same problem with the same shape of solution, and people still call it a semaphore in that case. See e.g. various npm packages.

Ok, it's good to know that there's precedent at least in the JS community for this usage, but I'm still not convinced that's what most people will think of first, e.g. the first sentence of the wiki article says "In computer science, a semaphore is a variable or abstract data type used to control access to a common resource by multiple threads...". I'm not saying we should treat wikipedia as gospel, but I do think it's indicative in this case of what most people's first idea of a semaphore would be.

Also per the readme the idea is that these would be shareable across threads (which would require some host integration, and underlying use of atomics), so it would be useful for the classical parallel case as well.

Thanks! I was able to find this after you mentioned it (using CTRL+F), but it would be great to see this mentioned in the first few paragraphs of the readme. No one picked up on this when we did our internal review of this proposal.

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

No branches or pull requests

2 participants