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.

Sign up for GitHub

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

Jump to bottom

Test the number of processes started by various shell snippets #254

Closed
andychu opened this issue Mar 19, 2019 · 3 comments
Closed

Test the number of processes started by various shell snippets #254

andychu opened this issue Mar 19, 2019 · 3 comments
Labels
devtools pending-release performance

Comments

@andychu
Copy link
Contributor

andychu commented Mar 19, 2019

This takes 3 processes in OSH and bash, but I thought OSH was optimized to use 2?

strace -o _tmp/cs_opt/osh/t  -ff -- osh -c 'echo $(ls)'

That is, one for osh itself, and then one for ls. The fork_external flag makes it so the subshell doesn't fork again? That might have been broken.

This is also related to shopt -s lastpipe, etc.

We could write a shell test framework that uses strace and counts the number of output files? Is there an easier way?
andychu pushed a commit that referenced this issue Mar 19, 2019
Fix crash in 'builtin command ls'.
a9f6b40 
I had put 'command' in an odd place in the Executor because I didn't
want to thread (fork_external, span_id) through so many functions.

But that caused a bug in the odd case of 'builtin command ls'.  Now
we're doing it the obvious way.  Everything fits together correctly.

In the future, we want to:

- have argv and span_id travel together.  The evaluated argv array needs
  location info.
- fork_external needs tests (issue #254).  Maybe make it a flag on the
  Executor instead of a parameter to so many methods.
@andychu
Copy link
Contributor Author

andychu commented Jul 4, 2019

Also test the number of syscalls with strace. Maybe do our own strace -c ?

andychu pushed a commit that referenced this issue Mar 27, 2020
[test/count-procs] Count the processes started by different shells.
b1d6de9 
Add comments in the code where I think extra processes are invoked.

Addresses issue #254.

Also:

- Minor translation fixes.  Down to 89 errors.
- Failing spec tests for 'proc return' problem.  Which is also causing a
  type error.
@andychu
Copy link
Contributor Author

andychu commented Mar 27, 2020 •
edited

Easy solution: use -ff -o ${case_id}-${sh}, then count the listings in that directory with Python

andychu pushed a commit that referenced this issue Mar 27, 2020
[test/count-procs] Count syscalls too.
49dc6da 
- Print a table of syscalls
- Sort horizontally by shell performance
- Add more test cases
- Clean up log output

Addresses issue #254.
andychu pushed a commit that referenced this issue Mar 27, 2020
[refactor] Rename test/count-procs -> test/syscall
282ea42 
Addresses issue #254.
@andychu
Copy link
Contributor Author

andychu commented Mar 27, 2020 •
edited

TODO:

  • assert no regressions
  • publish in toil
  • publish in /release/$VERSION

andychu pushed a commit that referenced this issue Mar 28, 2020
[test/syscall] Build yash from source and run tests on it.
f8c2fd4 
- Print comparison of OSH against the minimum, and against bash
- Assert the number of failures, so there aren't regressions.
- Add a few more test cases

Addresses issue #254.
andychu pushed a commit that referenced this issue Mar 29, 2020
[osh/cmd_exec] Optimize the number of processes started.
bcb0da5 
command.Simple now has a do_fork attribute which can be turned off.

Add more test cases.  Addresses issue #254.

Score:

- Our "score" is 60, compared with 78 for bash (the max) and 57 for yash
  (the min).
- We beat zsh which has a score of 62.
- We no longer start more processes than bash in any case.  We start
  FEWER in 17 out of 30 cases!
- Oil isn't the minimum in 4 out of 30 cases.  There's still at least
  one of these I think we should handle.

Other trivia:

- We lost the 'command date | wc -l' optimization, but no other shell
  does this!
- bash starts TWO more processes than other shells, for case 28.
- We also behave like yash in optimizing the same way whether we get -c,
  stdin, or file input.  Other shells only optimize -c.  TODO: publish
  these tests.
andychu pushed a commit that referenced this issue Mar 29, 2020
[performance] Optimize the AST for Subshell and CommandSub.
f76a7fc 
This enables one more optimization on test/syscall, so our score drops
to 59 from 60.

Related to issue #254.
@andychu andychu closed this as completed Apr 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
devtools pending-release performance
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@andychu