Skip to content

Comments

Auto-tuning aio parameters#1059

Merged
tjruwase merged 7 commits intomasterfrom
olruwase/aio_auto_tuner
May 10, 2021
Merged

Auto-tuning aio parameters#1059
tjruwase merged 7 commits intomasterfrom
olruwase/aio_auto_tuner

Conversation

@tjruwase
Copy link
Contributor

Performance scripts for auto-tuning/auto-generating aio parameters in deepspeed config.
Closing #998.

@stas00
Copy link
Collaborator

stas00 commented May 10, 2021

Great work, @tjruwase! That's much easier to use.

The only unintuitive part is --log_dir - it took me some time to figure out what default was used, as I needed to feed it to the 2nd script - which I figured out was .. Perhaps let's pick a default that is not . but some path? say logs ?

And then the second script probably should use that same log dir default and not ask for it? So all the user needs to run is:

python ./aio_bench_perf_sweep.py --io_dir io-test
python ./aio_bench_generate_param.py

or even have an optional script that runs both? Now that the benchmark part is really fast for easy-of-use probably combining the two?

Plus for --io-dir perhaps we can use a unique dir based on PID and delete it at the end of the run, or even using tempfile.TemporaryDirectory, so the user will just need to run a single script and then we don't need to think about log-dirs at all - again can use PID-based dir for safety.

python ./aio_bench_and_generate_param.py

@tjruwase
Copy link
Contributor Author

tjruwase commented May 10, 2021

@stas00, thanks for the feedback.

  1. --log_dir, I agree with using something other than . as default value. How about something more specific, like _aio_bench_logs?
  2. --io_dir, I think users do need to specify this so that the script can be run from a different folder than the one being tested. However, the script does create and delete a subfolder that contains the read and write files.

@stas00
Copy link
Collaborator

stas00 commented May 10, 2021

@stas00, thanks for the feedback.

1. --log_dir, I agree with using something other than `.` as default value. How about something more specific, like `_aio_bench_logs`?

That's perfect.

2. --io_dir, I think users do need to specify this so that the script can be run from a different folder than the one being tested. However, the script does create and delete a subfolder that contains the read and write files.

Ah, yes! we discussed that already - I forgot!

Should we rename it to match mnemonics better? e.g. nvme_dir

Also I'm not sure the description is clear:

  --io_dir IO_DIR       Directory in which to perform I/O tests. Normally 
                        where (NVMe) device is mounted.

That would suggest to me to use the mount point of the drive. Perhaps:

a writable sub-dir on an NVMe drive

? Not liking it 100% either...

@tjruwase
Copy link
Contributor Author

Yes, I will switch to nvme_dir. The only reason I did not use that is because as you observed previously, the library works on regular hard drives as well. But since that is not the intended use-case, nvme_dir is probably better.

Also, a writable sub-dir on an NVMe drive is clearly an improvement :). Will use.

Copy link
Contributor

@conglongli conglongli left a comment

Choose a reason for hiding this comment

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

General code style/logic looks good to me.

@tjruwase tjruwase merged commit 6124eb3 into master May 10, 2021
@mrwyattii mrwyattii deleted the olruwase/aio_auto_tuner branch July 7, 2023 02:40
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