<i18n dev> RFR: 8223490: Optimize search algorithm for determining default time zone
sean.coffey at oracle.com
Fri Sep 13 08:34:31 UTC 2019
Thanks for the review Roger. I run into compiler issues if I don't
declare the new function in the header file.
error: implicit declaration of function ‘isFileIdentical’
tz = isFileIdentical(buf, size, pathname);
If I try to declare it earlier in the main code, I also run into issues
since this new function calls other implicitly declared functions.
On 12/09/2019 18:47, Roger Riggs wrote:
> Hi Sean,
> In addition to Naoto's comments.
> The change to TimeZone_md.h should not be needed.
> A 'static' declaration doesn't need to be visible outside its source
> On 9/12/19 12:42 PM, naoto.sato at oracle.com wrote:
>> Hi Seán,
>> I like your approach to provide the fast path to determine the system
>> time zone. One general question is, is UTC/GMT the right set of fast
>> path candidates? Should we add some more common ones?
>> Other comments to the code:
>> - Should fast path search come after "dir" validation, i.e., line
>> - Line 126: "statbuf" can be removed.
>> - Line 134: 'i' is not size_of_something, so 'int' type should
>> suffice (and its initialization is done in the for-loop).
>> - Line 138: the fast path search should "continue" with the next
>> name, instead of "break".
>> - Line 142, 182: I'd wrap this line with parens for the if statement.
>> - Line 232-242: "pathname" is an argument to this function, so
>> freeing it inside the function seems odd. Also, no need to reset
>> dbuf/fd since they are no longer reused in the loop.
>> On 9/11/19 3:50 AM, Seán Coffey wrote:
>>> The current algorithm for determining the default timezone on
>>> (non-AIX) unix systems goes something like :
>>> 1. If TZ environment variable is defined, use it
>>> 2. else if /etc/timezone exists, use the value contained within it
>>> 3. else if /etc/localtime exists and is a symbolic link, use the
>>> name pointed to
>>> 4. else if /etc/localtime is a binary, find the first identical time
>>> zone binary file in /usr/share/zoneinfo/
>>> Step 4 is a bit crude in that the zoneinfo directory can contain
>>> over 1,800 files on today's systems. I'd like to change the logic so
>>> that common timezones are first checked for buffer matching before a
>>> full directory traversal is performed. It should be a performance
>>> gain and it should also lead to more consistent results for reasons
>>> outlined in the bug report.
>>> webrev: http://cr.openjdk.java.net/~coffeys/webrev.8223490/webrev/
More information about the core-libs-dev