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

docs: danda's first read edits #165

Merged
merged 3 commits into from
Jul 18, 2024
Merged

Conversation

dan-da
Copy link
Collaborator

@dan-da dan-da commented Jul 17, 2024

I read through the mdbook doc for the first time and made some edits that bring it more up to date with current codebase.

A few explanatory comments here:

re tarpc: HTTP/JSON implies that a client could connect from any language or a web browser, but that is not correct. We do not use http as transport. Rather we use tarpc's serde_transport to transmit JSON. Clients must also use rust+tarpc (unless they reverse engineer tarpc::serde_transport)

re threads/tasks/g in neptune-core overview: It is incorrect nomenclature to call these threads. They have never been threads. Rather they are tokio spawned tasks running on tokio's threadpool. They may run on the same or different operating system thread from the parent task, at tokio's discretion. Unfortunately there are still some comments and variables in the code that refer to tasks as threads, which may perpetuate the misconception. When I have time I will make a commit to correct this.

re canonical ordering of locks: That concept existed in order to acquire multiple locks in a certain order to avoid deadlock, but now there is only a single RwLock, that can only ever be write-acquired by one party at a time, so no need for ordering.

re deadlocks: now that we have a single lock over globalstate it's pretty hard to deadlock. you almost have to do it intentionally. updated docs with some simple rules.

Copy link
Contributor

@aszepieniec aszepieniec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Note that branch asz/transaction-consensus has more documentation in case you are interested.

Copy link
Member

@Sword-Smith Sword-Smith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good to me. I have two minor suggestions to the documentation of the software architecture.

docs/src/neptune-core/overview.md Show resolved Hide resolved
docs/src/neptune-core/overview.md Outdated Show resolved Hide resolved
@dan-da dan-da merged commit 7e64b1c into Neptune-Crypto:master Jul 18, 2024
3 checks passed
dan-da added a commit to dan-da/neptune-core that referenced this pull request Jul 18, 2024
replaces 'thread' with 'task' in struct names, variable names, comments,
and docs.

This is following up on:
Neptune-Crypto#165

quote:

  re threads/tasks/g in neptune-core overview:

  It is incorrect nomenclature to call these threads. They have never been
  threads. Rather they are tokio spawned tasks running on tokio's threadpool.
  They may run on the same or different operating system thread from the parent
  task, at tokio's discretion. Unfortunately there are still some comments and
  variables in the code that refer to tasks as threads, which may perpetuate
  the misconception.

This is now corrected.
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

Successfully merging this pull request may close these issues.

3 participants