Use multiprocessing for faster scanning

Use multiprocessing for faster scanning

[ekasper, 2013-09-24 13:43:32]

This is a temporary hack to make the scan tool faster by spawning multiple cert
parsing processes. I hope to fix ASN.1 parsing and make it all go away - but I
figured I'd check this in meanwhile for the unlikely event that someone else but
me finds it useful.

[Ben Laurie (Google), 2013-09-24 14:01:07]

LGTM.

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
File src/python/ct/client/tools/scan.py (right):

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
src/python/ct/client/tools/scan.py:5: import Queue
Unless I missed something, you are only using multiprocessing.Queue.

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
src/python/ct/client/tools/scan.py:82: 
Extra line.

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
src/python/ct/client/tools/scan.py:155: scan_process.start()
Why not start immediately after creation?

[Eran, 2013-09-24 15:20:37]

drive-by review...

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
File src/python/ct/client/tools/scan.py (right):

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
src/python/ct/client/tools/scan.py:96: self.blocking = blocking
nit: 'blocking' is somewhat ambiguous here. Seems like what it actually does is
wait for user input, so I suggest 'wait_for_user' or 'wait_for_user_ack'

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
src/python/ct/client/tools/scan.py:98: def process_entries(entry_queue,
message_queue):
nit: name message_queue to output_queue so it's clear output messages go there
(message_queue is ambiguous: unclear if it's for input or output)

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
src/python/ct/client/tools/scan.py:165: for _ in range(len(workers)):
You could get rid of the 'worker_stopped' messages by simply looking at the
entry_queue: When that's empty, it means each worker is at most calculating
'match(c)' for one certificate. You could just join() on the workers (that would
require timing out on entry_queue.get() in the worker)

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
src/python/ct/client/tools/scan.py:166: entry_queue.put((0, "STOP_WORKER"))
Sending '0' here indicates that perhaps a more structured message in the queue
is called for. Since the count is only used for progress report, I suggest
dropping it off entirely and so avoid using a more structured message.

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
src/python/ct/client/tools/scan.py:168: elif msg.msg == "WORKER_STOPPED":
use constants rather than raw strings.

[ekasper, 2013-09-24 15:35:05]

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
File src/python/ct/client/tools/scan.py (right):

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
src/python/ct/client/tools/scan.py:5: import Queue
On 2013/09/24 14:01:08, Ben Laurie (Google) wrote:
> Unless I missed something, you are only using multiprocessing.Queue.

Yup, leftovers. Removed.

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
src/python/ct/client/tools/scan.py:82: 
On 2013/09/24 14:01:08, Ben Laurie (Google) wrote:
> Extra line.

Removed.

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
src/python/ct/client/tools/scan.py:96: self.blocking = blocking
On 2013/09/24 15:20:37, Eran wrote:
> nit: 'blocking' is somewhat ambiguous here. Seems like what it actually does
is
> wait for user input, so I suggest 'wait_for_user' or 'wait_for_user_ack'

Renamed to 'wait_for_ack'.

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
src/python/ct/client/tools/scan.py:98: def process_entries(entry_queue,
message_queue):
On 2013/09/24 15:20:37, Eran wrote:
> nit: name message_queue to output_queue so it's clear output messages go there
> (message_queue is ambiguous: unclear if it's for input or output)

Done.

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
src/python/ct/client/tools/scan.py:155: scan_process.start()
On 2013/09/24 14:01:08, Ben Laurie (Google) wrote:
> Why not start immediately after creation?

Done.

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
src/python/ct/client/tools/scan.py:165: for _ in range(len(workers)):
On 2013/09/24 15:20:37, Eran wrote:
> You could get rid of the 'worker_stopped' messages by simply looking at the
> entry_queue: When that's empty, it means each worker is at most calculating
> 'match(c)' for one certificate. You could just join() on the workers (that
would
> require timing out on entry_queue.get() in the worker)

Seems to me this in fact complicates the flow: you'd have to re-inspect the
message queue after joining the workers to see if any of them produced any more
hits.

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
src/python/ct/client/tools/scan.py:166: entry_queue.put((0, "STOP_WORKER"))
On 2013/09/24 15:20:37, Eran wrote:
> Sending '0' here indicates that perhaps a more structured message in the queue
> is called for. Since the count is only used for progress report, I suggest
> dropping it off entirely and so avoid using a more structured message.

But I very much like to have the progress report.

https://codereview.appspot.com/13857043/diff/4001/src/python/ct/client/tools/...
src/python/ct/client/tools/scan.py:168: elif msg.msg == "WORKER_STOPPED":
On 2013/09/24 15:20:37, Eran wrote:
> use constants rather than raw strings.

Done.