[threeten-dev] TzdbZonRulesProvider and providerBind

[Stephen Colebourne]

On 28 January 2013 22:05, Xueming Shen <xueming.shen at oracle.com> wrote:
> While working on the "j.u.TimeZone uses tzdb.jar" code [1], I just
> realized the "supposedly to be the optimization helper" BoundProvider
> actually stands in the way of performance. It forces the rules to be
> loaded eagerly, with hundreds of BoundProvider objects flying in
> the startup stage. It basically disables any lazy-loading design/impl
> and possible provider hashing mechanism by eagerly loading the
> rules into ZoneRulesProvider level. Interestingly, the spec suggests
> that "this is suitable for providers that cann't change their contents
> during the lifetime of the JVM", exactly what the TzdbZoneRuleProvider
> is supposed to be, but "can't not change their contents during lifetime"
> does not necessarily mean "have to eagerly loading everything at
> start", given in most use scenario, only limited zones are used, it's
> really not a good strategy.

Its certainly wrong that it creates the rules object for everything at startup.

> Given its only benefit appears to replace two hash mapping lookups
> by one (also, I don't think it's a good idea for the spi abstract class
> to get into the business of how the individual provider takes care of
> their zoneid->rules implementation). I would suggest we just remove
> the mechanism.
>
> Here is webrev that
> (1) disabled the mechanism (I hope you agree to simply remove it)
> (2) replaced the id->index->byte[]/rules lookup to id -> byte[]/rules
>      (least, we don't have to do binary search )

I would have said that the binary search would be faster. It certainly
should be smaller in memory than a ConcurrentHashMap.
I you have shown it to be faster, then removing the index lookup is OK.

It would make sense for the JDK implementation to not have Version.
The JDK will only supply one version of the data, so it would be
faster/smaller to not have a class and hash lookup for the version. I
suggest the file format stays the same, and the loading code simply
loads the latest version.

I suspect that the "bind" approach makes less sense because it isn't
integrated into ZoneRegion as I thought it was (perhaps an unfinished
change of mine). The problem if you have many versions is that you
have to find the latest version. This can slow things down.

I think my intention was that ZoneRegion should store the provider,
not the rules, and that the bound provider was added so that the net
effect would be negligible.

This needs more thinking than is possible at midnight...

Stephen