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

Support new spawn #13

Merged
merged 36 commits into from
Oct 25, 2024
Merged

Support new spawn #13

merged 36 commits into from
Oct 25, 2024

Conversation

joii2020
Copy link
Contributor

@joii2020 joii2020 commented Sep 26, 2024

The simulator uses multithreading to simulate spawn, and each spawn will create a new thread.
After this change, need to use the matching ckb-std to use it in the CKB contract.

Modify

  • Support spawn (multithreading)
  • Add testcase
  • Add CI

@joii2020 joii2020 changed the title [WIP] Support new spawn Support new spawn Sep 29, 2024
src/global_data.rs Outdated Show resolved Hide resolved
PROCESS_CONTEXT_ID.with(|f| f.borrow().clone())
}

pub fn inherited_fds(&self) -> Vec<Fd> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

return by reference.


pub fn wait(&self) {
let (lock, cvar) = &*self.data;
let mut event = lock.lock().unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This name "event" is inconsistent: see notify.

src/spawn.rs Outdated
0
}

fn read(&self, fd: Fd, buf: *mut c_void, length: *mut usize) -> c_int {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If 100 bytes of data are written from one end of the pipe and the other end performs the following actions:

  1. Reads 30 bytes
  2. Reads 40 bytes
  3. Reads 30 bytes

The simulator is unable to handle this correctly.

}
}
impl TxID {
fn next(&mut self) -> Self {

Choose a reason for hiding this comment

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

Why do you modify the original data here, but clone a copy of the data as the return value at the same time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In many places, the next step is HashMap::insert. This way, the return is more convenient to use.

@@ -14,5 +14,13 @@ jobs:

steps:
- uses: actions/checkout@v3
- name: Install llvm 16
run: wget https://apt.llvm.org/llvm.sh && chmod +x llvm.sh && sudo ./llvm.sh 16 && rm llvm.sh
Copy link
Collaborator

Choose a reason for hiding this comment

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

use llvm-18

src/simulator_context.rs Show resolved Hide resolved
src/spawn.rs Outdated
pub extern "C" fn ckb_wait(pid: u64, code: *mut i8) -> c_int {
let pid: ProcID = pid.into();
if !get_cur_tx!().has_proc(&pid) {
return 5; // WaitFailure
Copy link
Collaborator

Choose a reason for hiding this comment

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

define the errors first, no magic numbers.

@joii2020 joii2020 requested a review from XuJiandong October 21, 2024 08:50
src/lib.rs Outdated
@@ -56,7 +66,7 @@ lazy_static! {
}

fn assert_vm_version() {
if SETUP.vm_version != 1 {
if SETUP.vm_version != 1 && SETUP.vm_version != 2 {

Choose a reason for hiding this comment

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

There may be version 3 in the future, use if SETUP.vm_version == 0 instead

@XuJiandong XuJiandong merged commit d8ec8f9 into nervosnetwork:main Oct 25, 2024
1 check passed
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