Skip to content
This repository was archived by the owner on Mar 28, 2022. It is now read-only.

Conversation

@jhurliman
Copy link
Contributor

This patch changes the logic in main.cxx (for platforms where echoprint-codegen can spawn multiple threads) to only spawn threads when more than one file is being fingerprinted. This results in a significant speedup for a common use case where echoprint-codegen is ran once per input file.

@ghost ghost assigned alnesbit Feb 21, 2012
src/main.cxx Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

A better location for this comment might be up by the #ifdef _WIN32 directive. It could also be that use_threads is false because the user is processing one file on a system which does, in fact, support threads.

@jhurliman
Copy link
Contributor Author

The current patch properly handles the case where the system supports threads but only one file is being processed (tested on OSX Lion with a single input file), but that's a good point about combining the #ifdef _WIN32 statements together. I'll submit a new pull request when I get a moment.

@jhurliman
Copy link
Contributor Author

I took another look at this patch and I didn't see another more obvious place for the use_threads variable. Putting it any earlier in the file means the count variable won't be initialized, breaking bool use_threads = (count > 1); for thread-supported platforms.

@alnesbit
Copy link
Contributor

Oh, I meant just for the comment, not the variable itself. (So it should be a trivial modification..?)

@jhurliman
Copy link
Contributor Author

Bumping this patch for consideration.

@alnesbit
Copy link
Contributor

alnesbit commented Aug 3, 2012

Thanks, @jhurliman! We're accepting this patch and it will be merged in.

@jhurliman
Copy link
Contributor Author

Just checking on the status of this

fluential pushed a commit to fluential/echoprint-codegen that referenced this pull request Oct 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants