[squeak-dev] The Inbox: Chronology-Core-ul.54.mcz

Levente Uzonyi leves at caesar.elte.hu
Wed May 6 00:39:28 UTC 2020


Hi Dave,

On Tue, 5 May 2020, David T. Lewis wrote:

> On Sun, May 03, 2020 at 06:45:52PM +0000, commits at source.squeak.org wrote:
>> Levente Uzonyi uploaded a new version of Chronology-Core to project The Inbox:
>> http://source.squeak.org/inbox/Chronology-Core-ul.54.mcz
>> 
>> ==================== Summary ====================
>> 
>> Name: Chronology-Core-ul.54
>> Author: ul
>> Time: 3 May 2020, 8:45:27.099465 pm
>> UUID: 770bbc91-fb33-46c2-a283-57f7163b672e
>> Ancestors: Chronology-Core-nice.52
>> 
>> Automatically update the time zone cached by the VM every 30 minutes:
>
>
> I am concerned that we are adding kludges on top of kludges, while
> overlooking a very simple underlying problem.
>
> The intent of primitiveUtcWithOffset is to answer, in a single atomic
> operation, the current time UTC along with the current GMT offset. It
> should be reported directly from the operating system, and not otherwise
> cached or manipulated in the VM. The intended use case is for getting the
> current system time and offset to be used in the creation of DateAndTime now,
> nothing else.
>
> The original and still correct implementation of ioUtcWithOffset for
> Unix is a direct call to gettimeofday(). On any reasonably modern Unix
> system it provides the two values directly in a single OS system call.
>
> /* implementation of ioLocalSecondsOffset(), defined in config.h to
> * override default definition in src/vm/interp.h
> */
> sqInt sqUnixUtcWithOffset(sqLong *microSeconds, int *offset)
> {
>  struct timeval timeval;
>  if (gettimeofday(&timeval, NULL) == -1) return -1;
>  time_t seconds= timeval.tv_sec;
>  suseconds_t usec= timeval.tv_usec;
>  *microSeconds= seconds * 1000000 + usec;
> #if defined(HAVE_TM_GMTOFF)
>  *offset= localtime(&seconds)->tm_gmtoff;
> #else
>  {
>    struct tm *local= localtime(&seconds);
>    struct tm *gmt= gmtime(&seconds);
>    int d= local->tm_yday - gmt->tm_yday;
>    int h= ((d < -1 ? 24 : 1 < d ? -24 : d * 24) + local->tm_hour - gmt->tm_hour);
>    int m= h * 60 + local->tm_min - gmt->tm_min;
>    *offset= m * 60;
>  }
> #endif
>  return 0;
> }
>
> The oscog code base does not have ioUtcWithOffset in platforms/Cross/vm/sq.h,
> and the actual implementation of the primitive uses a workaround involving
> ioUTCMicroseconds and ioLocalSecondsOffset rather than the single call to
> ioLocalSecondsOffset.
>
> I am personally responsible for putting that kludge into Cog in earlier days,
> and I apologize for my lapse in judgement. It is horrible, and that's my fault.
>
> It now looks to me like we are trying to have the image keep track of current
> GMT offset, and then inform the VM when it changes so that the VM can update
> its cached offset value, which it then turns around and uses for my kludgy
> workaround for the lack of an ioUtcWithOffset implementation.
>
> Maybe it is time to add a proper ioUtcWithOffset, and get rid of the various
> workarounds in both the VM and the image?

Eliot always says that asking the OS about the current time zone is too 
costly for something like DateAndTime now.
I haven't benchmarked the C code, but I have a Smalltalk benchmark to 
demonstrate Eliot is right:


"primitiveUtcWithOffset uses the cached time zone offset"
a := Array new: 2.
[ Time primPosixMicrosecondClockWithOffset: a ] benchFor: 1 seconds. 
'28,200,000 per second. 35.5 nanoseconds per run. 0 % GC time.'.

"LocalePlugin's primitiveTimezoneOffset asks the OS about the offset via localtime()
https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/Cog/platforms/unix/plugins/LocalePlugin/sqUnixLocale.c#L675 "

l := Locale current.
[ l primTimezone ] benchFor: 1 seconds.
'773,000 per second. 1.29 microseconds per run. 0 % GC time.'


The latter takes 363 times longer according to this benchmark.


Levente

>
> Dave


More information about the Squeak-dev mailing list