This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| class ThreadPool | |
| attr_accessor :threads, :max_size | |
| def initialize max_size=4 | |
| @max_size = max_size | |
| @threads = [] | |
| end | |
| def start *args, &block | |
| while(threads.count>=max_size) do | |
| threads.select!{ |thr| !thr.stop? } | |
| sleep(0.1) | |
| end | |
| threads << Thread.new(args) do |args| | |
| begin | |
| args = args[0] if args.length==1 | |
| yield args | |
| rescue Exception => e | |
| puts e.message | |
| puts e.backtrace | |
| end | |
| end | |
| end | |
| def join | |
| threads.map(&:join) | |
| end | |
| end | |
| # p = ThreadPool.new | |
| # 1.upto(10) do |i| | |
| # p.start(i) do |a| | |
| # $stdout.flush | |
| # puts "p #{a}" | |
| # end | |
| # end | |
| # p.join |
keep it simple and standardized 
Ok, I’m sorry, but this tiny library has so many problems:
* Line 10-13 is a busy loop – slightly better because you use sleep, but depending on what you do, 0.1s is a *very* long time (it’s more than 300 million clock ticks for a modern CPU)
* Line 10-13 is a race condition (select! on a shared array)
* Line 10, don’t use Array#count, that’s for counting things, use #size or #length (which you later even use)
* Line 15, race condition again (<< on a shared array)
* Line 17, why do you mess with the passed arguments? Pass them on as provided: yield(*args)
* Line 20, do NOT, really, NEVER rescue foreign exceptions, and worst of all, don't rescue `Exception` itself. It's the block's responsibility to rescue its exceptions. It certainly can deal better with it than randomly print
* Line 28-30, race condition again (map on a shared array) – this is potentially the worst race condition as it has the highest probability to happen – given that it's a blocking operation
`threads` array is not being accessed across threads but only in main thread, so no race condition around it. `threads` will get called either directly from main thread or when main thread calls `start`, even inside start, its getting accessed outside thread. `ThreadPool` objects belong to main thread.
If ThreadPool#threads is only accessed from a single thread, then yes, none of the race conditions applies. But you don’t ensure that in any way, and you don’t document either that only a single thread should operate the pool.
A good thread pool IMO is thread-safe and can be operated from any thread.
Two other thoughts:
* ThreadPool#threads= should IMO not exist (exists via attr_accessor :threads) – there’s no reason to reassign @threads
* Given that (again, IMO) you should neither rescue exceptions nor mess with the arguments, Line 15-25 can be shortened to just `threads << Thread.new(*args, &block)`
I hope that helps :)
Is there really a need for using size or length over count – following excerpt is from Ruby 2.0 documentation:
Obtaining Information about an Array
Arrays keep track of their own length at all times. To query an array about the number of elements it contains, use length, count or size.
browsers = [‘Chrome’, ‘Firefox’, ‘Safari’, ‘Opera’, ‘IE’]
browsers.length #=> 5
browsers.count #=> 5
I think, I am habitual to call .count, by having known some old basics of rails
Array#count works, I didn’t dispute that. And while the performance difference is likely academical only (since rubys method call overhead will beat the two if’s in the C code by a large amount), it’s still IMO bad style. If you still want to use it, at least be consistent (the code above once uses count, once length).
ohh, I did not notice that :)
So you mean one rubiest must call either count or length or size, and not mixture on a single project or for entire life :)
Btw, I had happened to use `args.length`, as I saw somebody explained variable arguments on some page when googled for it, and it had used .length there. I googled it because I did not use them recently.
And yes, in the latest version of my gist I passed *args directly to the yield. So no args.length checking.
:)
Denis Major:
You tell me. This is the source code for an array in ruby. Search for rb_ary_length and rb_ary_count and see the difference.
http://svn.ruby-lang.org/cgi-bin/viewvc.cgi/tags/v1_9_2_0/array.c?view=markup
Array#length and Array#size are synonyms, but Array#count is a somewhat different beast. It is intended to count for objects that match a given value or for which a given block returns true. Pickaxe 1.9 claims that it returns an enumerator if neither an argument or block is passed, but that doesn’t appear to be the case in 2.0 and the docs for 1.9.2 and 1.9.3 match the docs for 2.0.