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

Multithreaded startrail processing. #649

Merged
merged 4 commits into from
Oct 14, 2021
Merged

Multithreaded startrail processing. #649

merged 4 commits into from
Oct 14, 2021

Conversation

ckuethe
Copy link
Collaborator

@ckuethe ckuethe commented Oct 14, 2021

Inspired by @sebmueller. This splits up the input images into batches of work
more or less evenly divided across the number of processors.

Two new flags are added:

  • -n / --nice to make the workers be nice. default = 10
  • -m / --max-threads to control the number of worker threads. default = ncpus

Allows me to process a night of startrails on my workstation in under 3.5s.
On my pi3b+, it'll use between 350% - 395% CPU without making it feel slow

root@skycam:/home/allsky/src# time ./startrails -b 0.255 -d ../images/20210912/ -e jpg -o /tmp/junk.jpg -s 1280x960
Minimum: 0.0265446 maximum: 0.804403 mean: 0.143959 median: 0.137787

real	5m5.951s
user	11m22.243s
sys	1m5.347s

root@skycam:/home/allsky/src# time ./startrails -b 0.255 -d ../images/20210912/ -e jpg -o /tmp/junk2.jpg -s 1280x960 --max-threads 1
Minimum: 0.0265446 maximum: 0.804403 mean: 0.143959 median: 0.137787

real	12m8.758s
user	11m3.073s
sys	0m54.200s

root@skycam:/home/allsky/src# md5sum /tmp/junk*
686e577252e03c5ebf9d569f20acd1e6  /tmp/junk2.jpg
686e577252e03c5ebf9d569f20acd1e6  /tmp/junk.jpg

Fixes #628

Inspired by @sebmueller. This splits up the input images into batches of work
more or less evenly divided across the number of processors.

Two new flags are added:
* -n / --nice to make the workers be nice. default = 10
* -m / --max-threads to control the number of worker threads. default = ncpus

Allows me to process a night of startrails on my workstation in under 3.5s.
On my pi3b+, it'll use between 350% - 395% CPU without making it feel slow

```
root@skycam:/home/allsky/src# time ./startrails -b 0.255 -d ../images/20210912/ -e jpg -o /tmp/junk.jpg -s 1280x960
Minimum: 0.0265446 maximum: 0.804403 mean: 0.143959 median: 0.137787

real	5m5.951s
user	11m22.243s
sys	1m5.347s

root@skycam:/home/allsky/src# time ./startrails -b 0.255 -d ../images/20210912/ -e jpg -o /tmp/junk2.jpg -s 1280x960 --max-threads 1
Minimum: 0.0265446 maximum: 0.804403 mean: 0.143959 median: 0.137787

real	12m8.758s
user	11m3.073s
sys	0m54.200s

```

Fixes #628
@ckuethe
Copy link
Collaborator Author

ckuethe commented Oct 14, 2021

Running on all cores but at nice 10 was entirely arbitrary. But if I had a multiprocessor machine I was throwing at an embarrassingly parallel problem like this, I'd want to use all available cores. The nice level should keep this from making your machine feel laggy but as long as you're not trying to do too much other stuff you'll get a nice speed-up.

Copy link
Collaborator

@EricClaeys EricClaeys left a comment

Choose a reason for hiding this comment

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

Line 260: if ((tmp >= 1) || (tmp < cf->num_threads)).
Should it be "&&" instead? tmp of 1000 and -1 are both valid with an "||".

I suggest adding an error msg if tmp is out of range.

Line 265, " | -s] [-m ] [-n ]"
says -m and -n are optional, but lines 193 and 194 say "required_argument". Which is correct?

Line 287, suggest changing "nproc" to "number of processors" since many users won't know what "nproc" means.

@ckuethe
Copy link
Collaborator Author

ckuethe commented Oct 14, 2021

Check the getopt manage. required_argument means that a value/parameter is required for that option, not that it is necessary to specify this flag.

Nproc is fine. Any who knows enough about the computer architecture to know that they have multiple processors/cores and is able to accurately assess the impact of these options will understand nproc.

Copy link
Collaborator

@EricClaeys EricClaeys left a comment

Choose a reason for hiding this comment

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

Should "-m 0" be valid and mean "all", as the help message suggests (0 = nproc)?
If so, the "else" on line 242 should probably be else if (tmp != 0)

@ckuethe
Copy link
Collaborator Author

ckuethe commented Oct 14, 2021

I decided that -m 0 is silly because that would literally mean "do no work" so I just change the help to say that not specifying this means to use all cpus

@ckuethe ckuethe merged commit f562b4c into AllskyTeam:master Oct 14, 2021
@ckuethe ckuethe deleted the multithread branch October 14, 2021 04:13
@ckuethe ckuethe mentioned this pull request Oct 16, 2021
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.

using multiple cores for calculationg startrails / keogram
2 participants