Introducing the *TaskRunner interfaces

[Fred Akalin]

Hey guys,

I just landed r121999, which adds the TaskRunner, SequencedTaskRunner, and SingleThreadTaskRunner interfaces, and makes MessageLoopProxy inherit from SingleThreadTaskRunner.

What are these interfaces?

In brief:

• TaskRunner has methods for posting tasks, but provides no guarantees as to when, in what order, and on what thread these tasks are run. (Think of a thread pool.)
• SequencedTaskRunner subclasses TaskRunner to provide ordering guarantees, but not thread guarantees. (Think of a sequenced thread pool with a fixed sequence token.)
• SingleThreadTaskRunner subclasses SequencedTaskRunner to guarantee that tasks are run on a single thread. (Think of a message loop.)

Why were they created?

The *TaskRunner interfaces grew out of Brett's "The FILE thread is dead" thread, as it became clear that there was a need to decouple task posting from the mechanics of how each task will be run.  Also, there's always been a long-standing need to be able to dependency-inject task posting into our various classes and functions, especially for testing: how many times have you struggled with instantiating the right set of MessageLoop, Thread, and BrowserThread objects to be able to unit test your class properly?

OK, I'm sold! When can I start using these interfaces?

Right away!  There's still plenty of work left to be done (see the next section), but there are some easy things you can do:

• Convert uses of MessageLoopProxy to use SingleThreadTaskRunner, which is a drop-in replacement.  Direct use of MessageLoopProxy is now deprecated (except for calling current() to get a pointer to the current one, of course).
• In a similar vein, convert classes that directly use MessageLoop/BrowserThread/etc. to instead use one of the *TaskRunner interfaces via dep-injection.  It'll make it way more testable!  (Once the test implementations are written, that is.)

If you're feeling adventurous and want to start using non-trivial implementations of the *TaskRunner interfaces right away, shoot me an e-mail with your use case and I'll let you know when there's something ready for you to use.

Also, if you feel like helping out, there's a list of bugs below for remaining work. :)

When should I use each interface?

• Do you need your tasks to be run on the same thread?  Use SingleThreadTaskRunner.
• Do your tasks operate on objects that implement NonThreadSafe or similar?  Use SingleThreadTaskRunner.
• Do you need your tasks to be ordered, but don't really care if they're run on different threads?  Use SequencedTaskRunner.
• Do you want your tasks to be run on a thread pool, but need them to be ordered?  Use SequencedTaskRunner.
• Do you not care how your tasks are ordered or on what thread they're run on?  Use TaskRunner.
• Do you want your tasks to be run on a thread pool and don't care about ordering?  Use TaskRunner.

What work remains to be done?

• Writing a TaskRunner wrapper around WorkerPool (114329).
• Writing a SequencedTaskRunner wrapper around a SequencedWorkerPool with a fixed sequence token (114330).
• Writing *TaskRunner implementations for testing (114331).
• Writing specification tests for *TaskRunner implementations (114327).
• Writing an equivalent of base::NonThreadSafe for *TaskRunner (114332).

These tasks are marked with the "TaskRunner" label, as well as any other ones I think of later.  I plan to start on one of these, but I'll appreciate any help with the others!

Comments and questions are welcome.

- Fred

(Special thanks to darin@ and willchan@ for reviewing the implementation and hashing out the details.) 

[John Abd-El-Malek]

This is very cool, thanks so much for doing this. I'm sure it'll cleanup our code a lot! Some comments below.

On Tue, Feb 14, 2012 at 11:01 PM, Fred Akalin <aka...@chromium.org> wrote:

Hey guys,

I just landed r121999, which adds the TaskRunner, SequencedTaskRunner, and SingleThreadTaskRunner interfaces, and makes MessageLoopProxy inherit from SingleThreadTaskRunner.

What are these interfaces?

In brief:

• TaskRunner has methods for posting tasks, but provides no guarantees as to when, in what order, and on what thread these tasks are run. (Think of a thread pool.)
• SequencedTaskRunner subclasses TaskRunner to provide ordering guarantees, but not thread guarantees. (Think of a sequenced thread pool with a fixed sequence token.)
• SingleThreadTaskRunner subclasses SequencedTaskRunner to guarantee that tasks are run on a single thread. (Think of a message loop.)

Why were they created?

The *TaskRunner interfaces grew out of Brett's "The FILE thread is dead" thread, as it became clear that there was a need to decouple task posting from the mechanics of how each task will be run.  Also, there's always been a long-standing need to be able to dependency-inject task posting into our various classes and functions, especially for testing: how many times have you struggled with instantiating the right set of MessageLoop, Thread, and BrowserThread objects to be able to unit test your class properly?

OK, I'm sold! When can I start using these interfaces?

Right away!  There's still plenty of work left to be done (see the next section), but there are some easy things you can do:

• Convert uses of MessageLoopProxy to use SingleThreadTaskRunner, which is a drop-in replacement.  Direct use of MessageLoopProxy is now deprecated (except for calling current() to get a pointer to the current one, of course).
• In a similar vein, convert classes that directly use MessageLoop/BrowserThread/etc. to instead use one of the *TaskRunner interfaces via dep-injection.  It'll make it way more testable!  (Once the test implementations are written, that is.)

I see the benefit if someone is adding a test to a self-contained piece of code to convert that code to use these interfaces. But I'm a little worried that mass conversion of code that uses BrowserThread::PostTask(BrowserThread::X... to then make that object take an interface and change all the dependent code will just be busy work without much benefit, and will just make the code slightly harder to follow (i.e. now pass file/ io interface everywhere). 

If you're feeling adventurous and want to start using non-trivial implementations of the *TaskRunner interfaces right away, shoot me an e-mail with your use case and I'll let you know when there's something ready for you to use.

Also, if you feel like helping out, there's a list of bugs below for remaining work. :)

When should I use each interface?

• Do you need your tasks to be run on the same thread?  Use SingleThreadTaskRunner.
• Do your tasks operate on objects that implement NonThreadSafe or similar?  Use SingleThreadTaskRunner.
• Do you need your tasks to be ordered, but don't really care if they're run on different threads?  Use SequencedTaskRunner.
• Do you want your tasks to be run on a thread pool, but need them to be ordered?  Use SequencedTaskRunner.

I hope no one uses the thread pool in the meantime until this is done! 

• Do you not care how your tasks are ordered or on what thread they're run on?  Use TaskRunner.
• Do you want your tasks to be run on a thread pool and don't care about ordering?  Use TaskRunner.

What work remains to be done?

• Writing a TaskRunner wrapper around WorkerPool (114329).
• Writing a SequencedTaskRunner wrapper around a SequencedWorkerPool with a fixed sequence token (114330).
• Writing *TaskRunner implementations for testing (114331).
• Writing specification tests for *TaskRunner implementations (114327).
• Writing an equivalent of base::NonThreadSafe for *TaskRunner (114332).

These tasks are marked with the "TaskRunner" label, as well as any other ones I think of later.  I plan to start on one of these, but I'll appreciate any help with the others!

Comments and questions are welcome.

- Fred

(Special thanks to darin@ and willchan@ for reviewing the implementation and hashing out the details.) 

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Michael Nordman]

Cool beans :)

I added another another crbug with the TaskRunner label.

Modify BrowserMessageFilter::OverrideThreadForMessage to return a
TaskRunner instead of a well known BrowserThread::ID
http://code.google.com/p/chromium/issues/detail?id=114474

[John Abd-El-Malek]

On Wed, Feb 15, 2012 at 1:41 PM, Michael Nordman <mich...@google.com> wrote:

Cool beans :)

I  added another another crbug with the TaskRunner label.

Modify BrowserMessageFilter::OverrideThreadForMessage to return a
TaskRunner instead of a well known BrowserThread::ID
http://code.google.com/p/chromium/issues/detail?id=114474

This is the sort of changes I was worried about in my earlier email :) i.e. it's not clear to me what benefit we would have to change the message filters to not use BrowserThread::FOO directly. BrowserMessageFilters work with IO threads, and if code in them is sending something to a different thread, it's probably doing that for a good reason and it won't work without changes on any thread.

[William Chan (陈智昌)]

On Wed, Feb 15, 2012 at 1:27 PM, John Abd-El-Malek <j...@chromium.org> wrote:

This is very cool, thanks so much for doing this. I'm sure it'll cleanup our code a lot! Some comments below.

On Tue, Feb 14, 2012 at 11:01 PM, Fred Akalin <aka...@chromium.org> wrote:

Hey guys,

I just landed r121999, which adds the TaskRunner, SequencedTaskRunner, and SingleThreadTaskRunner interfaces, and makes MessageLoopProxy inherit from SingleThreadTaskRunner.

What are these interfaces?

In brief:

• TaskRunner has methods for posting tasks, but provides no guarantees as to when, in what order, and on what thread these tasks are run. (Think of a thread pool.)
• SequencedTaskRunner subclasses TaskRunner to provide ordering guarantees, but not thread guarantees. (Think of a sequenced thread pool with a fixed sequence token.)
• SingleThreadTaskRunner subclasses SequencedTaskRunner to guarantee that tasks are run on a single thread. (Think of a message loop.)

Why were they created?

The *TaskRunner interfaces grew out of Brett's "The FILE thread is dead" thread, as it became clear that there was a need to decouple task posting from the mechanics of how each task will be run.  Also, there's always been a long-standing need to be able to dependency-inject task posting into our various classes and functions, especially for testing: how many times have you struggled with instantiating the right set of MessageLoop, Thread, and BrowserThread objects to be able to unit test your class properly?

OK, I'm sold! When can I start using these interfaces?

Right away!  There's still plenty of work left to be done (see the next section), but there are some easy things you can do:

• Convert uses of MessageLoopProxy to use SingleThreadTaskRunner, which is a drop-in replacement.  Direct use of MessageLoopProxy is now deprecated (except for calling current() to get a pointer to the current one, of course).
• In a similar vein, convert classes that directly use MessageLoop/BrowserThread/etc. to instead use one of the *TaskRunner interfaces via dep-injection.  It'll make it way more testable!  (Once the test implementations are written, that is.)

I see the benefit if someone is adding a test to a self-contained piece of code to convert that code to use these interfaces. But I'm a little worried that mass conversion of code that uses BrowserThread::PostTask(BrowserThread::X... to then make that object take an interface and change all the dependent code will just be busy work without much benefit, and will just make the code slightly harder to follow (i.e. now pass file/ io interface everywhere). 

+1

If you're feeling adventurous and want to start using non-trivial implementations of the *TaskRunner interfaces right away, shoot me an e-mail with your use case and I'll let you know when there's something ready for you to use.

I am very excited for net/ to move away from WorkerPool and use TaskRunner instead, backed by SequencedWorkerPool. That way I can delete the embarrassingly ugly worker_pool_linux.cc (now _posix.cc) code I wrote when I first joined chrome-team and didn't know what I was doing.

 

Also, if you feel like helping out, there's a list of bugs below for remaining work. :)

When should I use each interface?

• Do you need your tasks to be run on the same thread?  Use SingleThreadTaskRunner.
• Do your tasks operate on objects that implement NonThreadSafe or similar?  Use SingleThreadTaskRunner.
• Do you need your tasks to be ordered, but don't really care if they're run on different threads?  Use SequencedTaskRunner.
• Do you want your tasks to be run on a thread pool, but need them to be ordered?  Use SequencedTaskRunner.

I hope no one uses the thread pool in the meantime until this is done! 

• Do you not care how your tasks are ordered or on what thread they're run on?  Use TaskRunner.
• Do you want your tasks to be run on a thread pool and don't care about ordering?  Use TaskRunner.

What work remains to be done?

• Writing a TaskRunner wrapper around WorkerPool (114329).
• Writing a SequencedTaskRunner wrapper around a SequencedWorkerPool with a fixed sequence token (114330).
• Writing *TaskRunner implementations for testing (114331).
• Writing specification tests for *TaskRunner implementations (114327).
• Writing an equivalent of base::NonThreadSafe for *TaskRunner (114332).

These tasks are marked with the "TaskRunner" label, as well as any other ones I think of later.  I plan to start on one of these, but I'll appreciate any help with the others!

Comments and questions are welcome.

- Fred

(Special thanks to darin@ and willchan@ for reviewing the implementation and hashing out the details.) 

Thanks for actually implementing everything! Darin and I have been talking about this for awhile now (looks like 7 months or so? https://groups.google.com/a/chromium.org/group/chromium-dev/browse_thread/thread/adc4f8932a0ff8be/9785497a4c1bed10?lnk=gst&q=taskrunner), but as usual, I can only talk and don't get much done. I'm grateful you made it actually happen :)

[Fred Akalin]

On Wed, Feb 15, 2012 at 1:27 PM, John Abd-El-Malek <j...@chromium.org> wrote:

I see the benefit if someone is adding a test to a self-contained piece of code to convert that code to use these interfaces. But I'm a little worried that mass conversion of code that uses BrowserThread::PostTask(BrowserThread::X... to then make that object take an interface and change all the dependent code will just be busy work without much benefit, and will just make the code slightly harder to follow (i.e. now pass file/ io interface everywhere). 

Yeah, I'm not advocating mass conversion for its own sake.  But if you foresee that a class that uses, say, the FILE thread will eventually want to post tasks on the replacement worker pool, then it might make sense to make that class use TaskRunner now.  Basically, weigh the advantages/disadvantages and use your best judgment. :) 

I hope no one uses the thread pool in the meantime until this is done! 

Yeah, I'm currently working on writing implementations for SequencedWorkerPool etc.  So it's probably best to wait until that lands, if you want to use the worker pool.