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

Behavior when runtime < sample-interval #36

Closed
yarikoptic opened this issue May 30, 2024 · 4 comments · Fixed by #37
Closed

Behavior when runtime < sample-interval #36

yarikoptic opened this issue May 30, 2024 · 4 comments · Fixed by #37
Assignees

Comments

@yarikoptic
Copy link
Member

using

version

time ls
CONTRIBUTING.rst  README.md  pyproject.toml  setup.cfg  smoke-tests.sh*  src/  test/  test_logs.py*  test_script.py*  tox.ini  venvs/
LC_COLLATE=POSIX ls -bCF --color=auto --hyperlink=auto  0.00s user 0.00s system 81% cpu 0.003 total

so it virtually takes no time to do ls but with duct:

time duct ls
-----------------------------------------------------
duct is executing ls...

Log files will be written to .duct/logs/2024.05.30T16.15.23-209921_
-----------------------------------------------------
CONTRIBUTING.rst
pyproject.toml
README.md
setup.cfg
smoke-tests.sh
src
test
test_logs.py
test_script.py
tox.ini
venvs

-----------------------------------------------------
                    duct report
-----------------------------------------------------
Exit Code: 0
Command: ls
Wall Clock Time: 1.095590591430664
Number of Processes: 0
Log files location: .duct/logs/2024.05.30T16.15.23-209921_
duct ls  0.12s user 0.08s system 16% cpu 1.190 total

over a second.

@asmacdo
Copy link
Member

asmacdo commented May 30, 2024

+1

@asmacdo asmacdo changed the title Should not incur unnecessary delay Behavior when runtime < sample-interval Jun 3, 2024
@asmacdo
Copy link
Member

asmacdo commented Jun 3, 2024

When running ls or other very fast commands, the default values of 60 second report interval and 1 second sample interval don't really make sense.

Ill fix this with docs for now, lmk if you want some other behavior.

asmacdo added a commit to asmacdo/duct that referenced this issue Jun 3, 2024
@yarikoptic
Copy link
Member Author

I want behavior which makes sense for a user. IMHO it should be

  • try sampling first time right after starting a program. If too quick and nothing collected, just report that.
  • if exits before acquiring full set of 60 samples, it is ok -- summarize based on what is collected so far.

@yarikoptic
Copy link
Member Author

another fresh sample with current version of #37 : full run/output
❯ duct ls
duct is executing ls...
Log files will be written to .duct/logs/2024.06.04T16.45.28-850256_
CONTRIBUTING.rst
__pycache__
pyproject.toml
README.md
setup.cfg
smoke-tests.sh
src
test
test_logs.py
test_script.py
tox.ini
venvs

Exit Code: 0
Command: ls
Log files location: .duct/logs/2024.06.04T16.45.28-850256_
Wall Clock Time: 2.954258918762207
Memory Peak Usage: unknown%
CPU Peak Usage: unknown%

so it gives Wall Clock Time: 2.954258918762207 . IMHO should not wait for that long and Wall Clock Time should be from the point we started the process until it finished up (here -- milliseconds).

asmacdo added a commit to asmacdo/duct that referenced this issue Jun 6, 2024
1. No need to wait on join() the monitoring thread, it is safely killed
   when duct process exits.
2. pass when ps fails (because inner process has finished)
3. Collect the first sample immediately rather than waiting for
   <sample-interval>

Fixes con#36
@asmacdo asmacdo added this to the Initial Release milestone Jun 6, 2024
asmacdo added a commit to asmacdo/duct that referenced this issue Jun 6, 2024
Leaving out join() isn't ideal, the thread ends up in a race condition
with test cleanup

Related: con#36
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 a pull request may close this issue.

2 participants