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

Remove or default-disable forkproxy waitpid workarounds #1261

Closed
sporksmith opened this issue Apr 5, 2021 · 0 comments · Fixed by #1353
Closed

Remove or default-disable forkproxy waitpid workarounds #1261

sporksmith opened this issue Apr 5, 2021 · 0 comments · Fixed by #1353
Assignees
Labels
Component: Main Composing the core Shadow executable Priority: Medium Important but not urgent Tag: Performance Related to improving shadow's run-time

Comments

@sporksmith
Copy link
Contributor

To address #1134, we implemented a workaround that uses proxy threads to fork the plugin threads (keeping the plugins out of the worker child list), and ptrace-detaches plugins on every blocking syscall (keeping the tracee list at size 1). While this workaround is a performance improvement overall for large simulations, the ptrace detaching-and reattaching adds substantial linear overhead.

Since then we've implemented a more performant solution of scaling the number of worker threads with the number of hosts, so that each worker thread only has a small number of children/tracees. While this has some extra memory and context-switching overhead, it generally appears to be a more performant solution since it doesn't require ptrace detaching-and-reattaching. (It also enables work stealing without detaching and reattaching).

We should probably at least disable the old workarounds by default before release. It probably makes sense to go a step further and remove them entirely, but we may need more evaluation to know for sure. e.g. it might turn out that in some cases it's better to take the performance hit of detaching and reattaching to avoid the extra memory cost of using more worker threads.

@sporksmith sporksmith added Priority: Medium Important but not urgent Component: Main Composing the core Shadow executable Tag: Performance Related to improving shadow's run-time labels Apr 5, 2021
@sporksmith sporksmith added this to To do in Next Release v2.0.0 pre.1 via automation Apr 5, 2021
@robgjansen robgjansen self-assigned this May 11, 2021
@robgjansen robgjansen moved this from To do to In progress in Next Release v2.0.0 pre.1 May 11, 2021
@robgjansen robgjansen assigned sporksmith and unassigned robgjansen May 12, 2021
sporksmith pushed a commit to sporksmith/shadow that referenced this issue May 12, 2021
We want the default mode to be that Shadow calculates the number
of processors to use based on the number of hosts specified in the
configuration file. (See shadow#1251.) In this case, we do not want to
run the forkproxy workaround, because the fact that waitpid is O(n)
won't matter when each processor only runs a few pids.

Closes shadow#1261
sporksmith pushed a commit to sporksmith/shadow that referenced this issue May 12, 2021
We want the default mode to be that Shadow calculates the number
of processors to use based on the number of hosts specified in the
configuration file. (See shadow#1251.) In this case, we do not want to
run the forkproxy workaround, because the fact that waitpid is O(n)
won't matter when each processor only runs a few pids.

Closes shadow#1261
sporksmith pushed a commit to sporksmith/shadow that referenced this issue May 12, 2021
We want the default mode to be that Shadow calculates the number
of processors to use based on the number of hosts specified in the
configuration file. (See shadow#1251.) In this case, we do not want to
run the forkproxy workaround, because the fact that waitpid is O(n)
won't matter when each processor only runs a few pids.

Closes shadow#1261
sporksmith added a commit that referenced this issue May 12, 2021
Disable forkproxy by default

* Disable forkproxy / O(n)-waitpid-workaround by default
* Fix a shutdown bug in thread_ptrace that this exposed
    
Closes #1261
@sporksmith sporksmith linked a pull request May 12, 2021 that will close this issue
Next Release v2.0.0 pre.1 automation moved this from In progress to Done May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Main Composing the core Shadow executable Priority: Medium Important but not urgent Tag: Performance Related to improving shadow's run-time
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants