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

Process files in parallel to improve performance #113

Merged
merged 1 commit into from
Sep 12, 2014

Conversation

akoeplinger
Copy link
Contributor

Not sure if you want to take it, but this improved performance for me as well (i.e. it now saturates all my four cores instead of only one).

Sample improvement for our website (both timings with PR #112 applied) on my four core machine:

before after
real 1m26.940s real 0m25.262s
user 1m24.883s user 1m28.469s
sys 0m1.166s sys 0m2.610s

@gjtorikian
Copy link
Owner

Yesssssssss I ❤️ parallelization.

Is this really all it takes? It seems so simple.

@akoeplinger
Copy link
Contributor Author

Yep, at least the tests pass and it resulted in the same errors on my website. Props to the parallel gem for making this a breeze 🎉

@akoeplinger
Copy link
Contributor Author

Only problem could be if there's some shared state that the threads try to access/modify concurrently, but from what I glanced this is not the case?

@gjtorikian
Copy link
Owner

One suggestion I would make: I don't like libraries I write (that depend on other libraries) to disregard their options.

Could you set up a way to pass options along to Parallel, similar to what's being done with Typhoeus? That way any consumer of this can pass along in_processes or in_threads to their heart's content. I don't want html-proofer blocking that.

If you get that working, docs + tests for it would be grand! Otherwise I can pick it up. Thanks!

@akoeplinger
Copy link
Contributor Author

Hmm, I tried to add it, but I think something doesn't work, as I can't even pass a different Typhoeues option to the library. Output on master without any changes:

$ htmlproof _site --followlocation=false
/home/alexander/.rvm/gems/ruby-2.1.1/gems/mercenary-0.3.4/lib/mercenary/program.rb:30:in `go': invalid option: --followlocation=false (OptionParser::InvalidOption)
    from /home/alexander/.rvm/gems/ruby-2.1.1/gems/mercenary-0.3.4/lib/mercenary.rb:22:in `program'
    from /home/alexander/dev/html-proofer/bin/htmlproof:10:in `<top (required)>'
    from /home/alexander/.rvm/gems/ruby-2.1.1/bin/htmlproof:23:in `load'
    from /home/alexander/.rvm/gems/ruby-2.1.1/bin/htmlproof:23:in `<main>'
    from /home/alexander/.rvm/gems/ruby-2.1.1/bin/ruby_executable_hooks:15:in `eval'
    from /home/alexander/.rvm/gems/ruby-2.1.1/bin/ruby_executable_hooks:15:in `<main>'

@gjtorikian
Copy link
Owner

Yeah, I thought as much. Typhoeus is real picky about its options. Gonna merge this and work on the options in a separate PR.

gjtorikian added a commit that referenced this pull request Sep 12, 2014
Process files in parallel to improve performance
@gjtorikian gjtorikian merged commit bfa5f22 into gjtorikian:master Sep 12, 2014
@doktorbro
Copy link

I love it.

@akoeplinger akoeplinger deleted the parallelize branch September 12, 2014 12:33
@kansaichris
Copy link

So by default this will use two processes?

@gjtorikian
Copy link
Owner

By default it should use whatever Parallel's defaults are.

@kansaichris
Copy link

Hm, okay.

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.

4 participants