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

Be more optimistic when querying on Windows? #79

Open
dpc opened this issue Jul 3, 2019 · 3 comments · May be fixed by #93
Open

Be more optimistic when querying on Windows? #79

dpc opened this issue Jul 3, 2019 · 3 comments · May be fixed by #93

Comments

@dpc
Copy link

dpc commented Jul 3, 2019

As a part of cargo crev I was reviewing this crate in version 1.10.1 . All was good, but I have one idea.

It seems like on Windows this crate calls the GetLogicalProcessorInformation twice. Any reason not to start with a buffer of a fix size (eg. 64 entries)? Maybe this initial buffer could be on stack, so it wouldn't really cost anything. On most machines that would cut the number of calls to just 1.

@seanmonstar
Copy link
Owner

What you say sounds reasonable. I'm not too familiar with Windows programming, but I noticed the source links to an example on MSDN, and it looks like they also query once to get the size needed. Is the example naive?

@dpc
Copy link
Author

dpc commented Jul 3, 2019

Seems so to me. But I have no clue, and haven't wrote for or used Windows in more than a decade, so what do I know. :D

@DoumanAsh
Copy link

DoumanAsh commented Jan 12, 2020

More optimal approach would be to use first call with some stack array of hard-coded size.
If function fails with unsufficient memory, then it will update len to required one, in which case you could fallback to heap allocated vec.

This way in happy case the call will be only once.

@UnicodingUnicorn UnicodingUnicorn linked a pull request Jan 29, 2020 that will close this issue
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.

3 participants