-
Notifications
You must be signed in to change notification settings - Fork 157
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
Add an ARCHITECTURE.md document #302
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! I've nitpicks as usual. :) Don't forget to address the TODOs btw.
|
||
The architecture of [`smol-rs`]. | ||
|
||
This document describes the architecture of [`smol-rs`] and its crates on a high |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we follow 100 chars limit for lines in code so I think we should follow the same in markdown files. Short lines make the document look longer and people can be put off by size of the reading. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally I follow a limit for 80 chars in Markdown files, as some terminals rely on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These days? Which terminals? 😯
In any case, they'll have the same issues with the code. It's best to be consistent.
BTW, instead of putting all this in a separate doc, I think it should just be here. We'll need a new repo named |
Do you mean the |
AFAIK, you need to create a repository by the same name as the space (
People don't need to read all of it. I think visibility and discoverability is more important. Having said that, I wouldn't mind a different document on the front page but until someone write that, I think this should go there. |
It would be useful to have an ARCHITECTURE document that can be used by new contributors to get a "bird's eye view" of how things are arranged in smol. This commit adds a basic ARCHITECTURE.md document with the aim that it can be used by prospective contributors. So far only the lower-level items of the ecosystem have been described. Signed-off-by: John Nunley <dev@notgull.net>
Signed-off-by: John Nunley <dev@notgull.net>
6373d36
to
2e7d9b1
Compare
I've put an interim document on the frontpage for now. |
### [`parking`] | ||
|
||
[`parking`] is used to block threads until an arbitrary signal is received, or | ||
"parks" them. The [`std::thread::park`] API suffers from involving global state; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
", or parks them" is worded a bit confusingly (as "parks" is/should be associated to "block threads"), but this isn't obvious here.
In order to construct runtimes, you need to be able to store [`Waker`]s in a | ||
concurrent way. That way, different parties can simultaneously store [`Waker`]s | ||
to show interest in an event, while activators of the event can take that | ||
[`Waker`] can wake it. [`atomic-waker`] provides [`AtomicWaker`], which is a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[`Waker`] can wake it. [`atomic-waker`] provides [`AtomicWaker`], which is a | |
[`Waker`] to wake it. [`atomic-waker`] provides [`AtomicWaker`], which is a |
|
||
### [`concurrent-queue`] | ||
|
||
[`concurrent-queue`] is a fork of the [`crossbeam-queue`] crate. It provides a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be a good idea to explain why the fork happened.
optimized single-item queues, queues with a bounded capacity, and infinite | ||
queues with unbounded capacity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels redundant.
At a low level, [`async-task`] can be seen as putting a future on the heap and | ||
providing a handle that can be used by executors. The [`Runnable`] is the part | ||
of the task that represents the future to be run. When the [`Runnable`] is | ||
spawned to the existence by the coroutine indicating that it is ready, it can |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is difficult to read.
It would be useful to have an ARCHITECTURE document that can be used by
new contributors to get a "bird's eye view" of how things are arranged
in smol. This commit adds a basic ARCHITECTURE.md document with the aim
that it can be used by prospective contributors.
So far only the lower-level items of the ecosystem have been described.