search

ronshapiro / rxjava-priority-scheduler

An RxJava scheduler that incorporates priorities in scheduling tasks
55 stars 6 forks source link

Implementing PriorityScheduler based on ExecutorScheduler #4

Open NorseDreki opened 9 years ago

NorseDreki commented 9 years ago

@ronshapiro, what do you think about implementation of PriorityScheduler which is based on standard ExecutorScheduler?

There might be a PriorityBlockingQueue accepting PriorityExecutorActions as a repacement for ExecutorScheduler's ConcurrentLinkedQueue with ExecutorAction.

ronshapiro commented 9 years ago

Do you have anything in mind? What would be the added benefit?

FTR, here's a discussion in Guava about a possible implementation of a PriorityExecutorService - https://github.com/google/guava/issues/888 but I'm not sure if it will be implemented.

NorseDreki commented 9 years ago

For me the motivation is to keep it simple, go the shortest path and use what RxJava offers out of the box (ExecutorScheduler). I cannot name many benefits except that since it's standard -- it's tested; so does it also follows "the Worker contract" as mentioned in the discussion for this commit: 4f494499c456421433830e09e54e87018c7fca6b.

Here is a sketch how this could look like: upelsin@c50448cf91feca84cdcf1c0945126f6240436b21.

ronshapiro commented 9 years ago

I found there to be actually more complexity in this implementation and would still need tests since it only marginally uses the ExecutorSchedulerWorker. PriorityScheduler.withConcurrency(1) solves the comment in that commit about concurrency, but part of what makes this nice is giving you the flexibility to create multiple threads if you like (even if it's not the intended use). I do like your idea of using a ScheduledExecutorService for the executor so that it can handle schedule(Action0, long, TimeUnit) appropriately, I think that should be included.

NorseDreki commented 9 years ago

Your comments make sense, thanks for the discussion.