ThreeTen / threeten

This project was the home of code used to develop a modern date and time library for JDK8. Development has moved to OpenJDK and a separate backport project, threetenbp.
http://threeten.github.io/
191 stars 37 forks source link

Remove application defined Chronologies from Chronology.getAvailableChronologies #308

Closed RogerRiggs closed 11 years ago

RogerRiggs commented 11 years ago

The inclusion of application defined Chronologies in the method getAvailableChronologies requires that the ServiceLoader be consulted on every invocation; it is ClassLoader sensitive and therefore caller sensitive.
It is expected that new Chronologies will be installed/added to the base system and less frequently by the application. An application more likely to refer to the Chronology explicitly or can use the ServiceLoader directly to locate an application defined chronology by name. The functionality should be removed from getAvailableChronologies method. An alternative is to move it to a separate method dedicated to finding application provided chronologies.

jodastephen commented 11 years ago

The whole point of an "available" list is that it lists everything available. I would be OK with that being "at startup" if that makes a difference here.

RogerRiggs commented 11 years ago

Clarifying 'available' as applying to the system classloader would provide all the calendars available to all apps at startup and would remove the vagaries of using the callers classloader as part of the search.

jodastephen commented 11 years ago

I guess the problem with system classloader for this and time-zones is that it doesn't work well in Java EE where there is one class loader per app, which has the classes loaded. But I think its reasonable if it makes life simpler.

RogerRiggs commented 11 years ago

The concern was about the performance of getAvailableChronologies due to the scan of the ServiceLoader. A consistent changing to not search the ServiceLoader in getAvailable would also cause a change to Chronology.of(name) so it would not find Chronologies in the application classpath. On balance, the performance of getAvailableChronologies is sufficient and might be improved by some implementation technique. The behavior should stay the same.