Skip to content

Port LogicalProcessors to Rust#1434

Merged
sporksmith merged 15 commits intoshadow:mainfrom
sporksmith:c2rust-logicalprocessors
Jun 15, 2021
Merged

Port LogicalProcessors to Rust#1434
sporksmith merged 15 commits intoshadow:mainfrom
sporksmith:c2rust-logicalprocessors

Conversation

@sporksmith
Copy link
Copy Markdown
Contributor

This was mostly an experiment of using c2rust to translate a single C file.

Also fixes:

  • Attempted use of non-existent log level "message" when compiling with --use-perf-timers.
  • Failing to take 'quantity' into account when counting the number of hosts. This led to creating too few worker threads in simulations that had hosts with quantity > 1.

@sporksmith sporksmith force-pushed the c2rust-logicalprocessors branch from a8487e1 to 8cccc38 Compare June 15, 2021 02:26
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 15, 2021

Codecov Report

Merging #1434 (e396111) into main (01862fd) will decrease coverage by 2.40%.
The diff coverage is 78.32%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1434      +/-   ##
==========================================
- Coverage   56.43%   54.02%   -2.41%     
==========================================
  Files         141      137       -4     
  Lines       20165    20395     +230     
  Branches     5018     5155     +137     
==========================================
- Hits        11380    11019     -361     
- Misses       5880     6470     +590     
- Partials     2905     2906       +1     
Flag Coverage Δ
tests 54.02% <78.32%> (-2.41%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/main/core/manager.c 71.07% <ø> (+2.88%) ⬆️
src/main/host/descriptor/channel.c 28.72% <0.00%> (ø)
src/main/host/descriptor/tcp_retransmit_tally.cc 71.62% <ø> (+0.12%) ⬆️
src/main/host/syscall/uio.c 20.39% <0.00%> (-0.14%) ⬇️
src/main/host/syscall_condition.c 70.55% <ø> (+0.15%) ⬆️
src/main/host/syscall_handler.c 54.16% <ø> (+1.84%) ⬆️
src/main/host/thread.c 65.45% <ø> (+1.30%) ⬆️
src/main/host/thread_preload.c 0.00% <ø> (ø)
src/main/host/thread_ptrace.c 49.50% <ø> (-2.93%) ⬇️
src/main/host/tracker.c 38.64% <ø> (-0.21%) ⬇️
... and 90 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 613e971...e396111. Read the comment docs.

@sporksmith sporksmith requested a review from stevenengler June 15, 2021 13:55
Copy link
Copy Markdown
Contributor

@stevenengler stevenengler left a comment

Choose a reason for hiding this comment

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

Marked as "request changes" mostly for the performance effects of not making the idle timer a compile-time option, and for the basic_timer code not having a license (I'm not sure how important this is in reality).

@sporksmith sporksmith requested a review from stevenengler June 15, 2021 18:58
@sporksmith sporksmith changed the base branch from dev to main June 15, 2021 20:08
@sporksmith sporksmith force-pushed the c2rust-logicalprocessors branch from 1738161 to 2efeb91 Compare June 15, 2021 20:58
@sporksmith sporksmith enabled auto-merge June 15, 2021 20:58
When auto-configuring the number of worker threads to create, we were
counting the number of host *entries* instead of the number of hosts.
Each entry has a *quantity* field that we must account for.
For some reason the github CI seems to keep using a stale version of the
"shadow-fork" branch. Locking to a specific commit will ensure there's
no confusion about what version it's using, and prevent "silent" updates
via the vsnprintf repo.
@sporksmith sporksmith merged commit 32d1062 into shadow:main Jun 15, 2021
@sporksmith sporksmith deleted the c2rust-logicalprocessors branch June 15, 2021 22:29
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.

2 participants