Combine SimpleScheduler and SimpleTimer2 threads

[zzz]

Can we combine the backend ThreadPoolExecutors? for SimpleScheduler? and SimpleTimer2 to go from 8 threads to 4? We would keep the same classes and front-end interface, just execute them in the same pool.

[zab]

Do we know each and every Runnable that will ever execute on SimpleScheduler and SimpleTimer2? If not, the only way to find the answer is to try and see what happens. In the worst case scenario, this will cause a deadlock or a halt.

[zzz]

The proposal in the OP is messy.

Perhaps a better one is:

• Have ST2 support the SS methods, i.e.:
• Add the addEvent() and addPeriodicEvent() methods to ST2. Maintain the SimpleTimer?.TimedEvent? parameter
• In addEvent(), wrap the ST.TE class in a TimedEvent?
• In addPeriodicEvent(), wrap the ST.TE class in a PeriodicTimedEvent? extends TimedEvent?
• Convert all the in-tree calls from _context.simpleScheduler() to _context.simpleTimer2()
• Deprecate I2PAppContext.simpleScheduler()

[tuna]

Patch submitted in rev ef4752e434e9bdb82e9a41cb67e2a8ccead202a8 on i2p.i2p.tuna.tmp

[zzz]

review:

• in the new SimpleTimer2 methods, you must add the events to the correct SimpleTimer2, i.e. "this", not SimpleTimer2.getInstance()

• would be less code to just call the 3-arg addPeriodicEvent() from the 2-arg addPeriodcEvent(), as was done in SimpleScheduler?. You could then get rid of one of the PTE constructors.

• Add @since to new methods

• No need to deprecate private methods, although I suppose it doesn't hurt if it doesn't generate more warnings

• Not sure why you did an inline class on top of an abstract class instead of just making the PTE class concrete, to me it's less readable, but if you're used to doing Android coding it's awfully familiar. Up to you.

• If you haven't, you'll probably need to test with logging on to make sure the scheduled tasks are executed when expected. It may not be apparent if things don't work, since scheduled stuff is generally for data structure cleanups and other minor things.

• Probably need to analyze whether it's necessary to raise the number of ST2 threads to handle the additional work.

• I never did understand zab's comment 1 above. Hopefully you do, and have researched it to your satisfaction.

[zzz]

Also, refer users to the replacement ST2 in the SS javadocs/deprecations

[tuna]

1) Fixed
2) Fixed
3) Fixed
4) No new warnings generated, keeping the @deprecated
5) I can't say I have much of an Android background, but if keeping it an inline class is ok, I'll do that.
6) Remains to be done
7) Remains to be done
9) No, I don't understand zabs comment. Maybe he was referring to rescheduling from run method.
10) Fixed

[tuna]

Latest rev is: 4686588f843a7df14b001d6b9689d237547a675a

[tuna]

Latest rev is: 8584094b332cc5ad4f560d4765014125b7b911eb

[tuna]

I measured the difference in scheduled time and time at which event is executed under the patched and unpatched versions.

The test was run for 69minutes, catching all the events (~2700 of them) that were once handled by SS, on a 10k tunnel router.

patched
95th: 2.000000ms
Avg: 2.434004ms

unpatched
95th: 1.000000ms
Avg: 2.662817ms

[tuna]

Propagated from i2p.i2p.tuna.tmp to i2p.i2p

[zzz]

In 0.9.19-2.

Only plugin I see using SS is zzzot, to be fixed by me. See #1523