stephanenicolas / robospice

Repo of the Open Source Android library : RoboSpice. RoboSpice is a modular android library that makes writing asynchronous long running tasks easy. It is specialized in network requests, supports caching and offers REST requests out-of-the box using extension modules.
Apache License 2.0
2.95k stars 545 forks source link

Implement both retry policy and priorization #99

Closed stephanenicolas closed 11 years ago

stephanenicolas commented 11 years ago

Those are the only features that Volley offers and RS doesn't (yet).

http://www.youtube.com/watch?v=yhv8l9F44qo

rciovati commented 11 years ago

For priorization we could implement our custom ThreadPoolExecutor which internally uses a PriorityBlockingQueue (as examplained here). But if someone will use a custom ExecutorService will lose request priority support. What do you think? Which solutions are you thinking about?

dpsm commented 11 years ago

Should the priority order apply to requests across all spice managers within the processor or within each spice manager context avoiding an eager manager from delaying requests from another manager?

rciovati commented 11 years ago

IHMO priority management should be handled by the request processor.

dpsm commented 11 years ago

I agree. It should not be a big deal for an application to manage and define request priorities across it's components.

Makes things easier to implement and the effectiveness of the prioritization will increase doing it at the processor level.

stephanenicolas commented 11 years ago

I was thinking, originally, to implement it at both spice manager and request processor level.

I still think that's a good approach as it should not be such a big change to RS.

@David, I don't see how/why spice manager early ness could be taken into account.

I plan to release RS tomorrow and start working on this feature right away.

S. Le 19 mai 2013 19:28, "Riccardo Ciovati" notifications@github.com a écrit :

IHMO priority management should be handled by the request processor.

— Reply to this email directly or view it on GitHub.

dpsm commented 11 years ago

I guess it does not really apply what I said. If done at both levels the spice manager will prioritize the requests within its "scope" and the processor will prioritize the overall requests from many managers.

I think that it is a pretty good way to go with regards to this feature.

stephanenicolas commented 11 years ago

Ok, that's fine. If you guys agree, I will try to implement that tomorrow..

We might need beta testers though :)

dpsm commented 11 years ago

Sure! Let me know if you need any help. I might write some test cases for it as well.

How are u planning to shape the priority api. On the spice request class?

stephanenicolas commented 11 years ago

On a natural ordering of CachedSpiceRequest + delegation to SpiceRequest.

I would define a few constants, I don't like Volley's approach of an enum, I would prefer an integer. Devs would be more free and the constants could just be shorteners.

2013/5/19 David Sobreira Marques notifications@github.com

Sure! Let me know if you need any help. I might write some test cases for it as well.

How are u planning to shape the priority api. On the spice request class?

— Reply to this email directly or view it on GitHubhttps://github.com/octo-online/robospice/issues/99#issuecomment-18122065 .

Stéphane NICOLAS, OCTO Technology Développeur & Consultant Android / Java .......................................................... 50, Avenue des Champs-Elysées 75008 Paris +33 (0)6.26.32.34.09 www.octo.com - blog.octo.com www.usievents.com ...........................................................

stephanenicolas commented 11 years ago

The priorization system has been pushed. I had to dig quite deep into the code of ThreadedPoolExecutor and PriorityBlockingQueue to implement and understand tests. But it works in a predictible way now.

http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/6-b14/java/util/concurrent/ThreadPoolExecutor.java

There are some inherent limitations to this mechanism. For instance, the first runnable/callable passed to the executor doesn't go in the queue. Thus the priority mechanism will only apply when tasks are queued, and this happens when the number of current runners is exceeds the number of max thread in the pool size.

It can look like the implementation is not working, but it actually is and tests work at a rate of 100% (hopefully in CI on a slow emulator as well).

This feature gets all its meaning in the case you load A LOT of requests into RoboSpice. For instance, you load images in a listview AND some data from a Pojo. Then you can decide to give a small priority to images, and you will get your Pojo asap.

rciovati commented 11 years ago

I'm glad my idea made sense :)

stephanenicolas commented 11 years ago

Actually that was exactly the way I did it. :) Still, the spice manager's queue should also take priority into account that would limit the impact of the first N < #core priorisation problem.

And RetryPolicy + back off alg also remain. (Note to self ;) )