Project

General

Profile

Actions

Bug #13088

open

uniqtime() is too clever for its own good

Added by Patrick Mooney over 1 year ago. Updated 7 months ago.

Status:
New
Priority:
Normal
Assignee:
Category:
kernel
Start date:
Due date:
% Done:

0%

Estimated time:
Difficulty:
Bite-size
Tags:
Gerrit CR:

Description

As richlowe pointed out in the feedback for #7709, the uniqtime() function also has "optimized" time conversion logic which is meant to avoid costly integer division on older platforms.
This is another case where x86 could probably do the simple thing.


Related issues

Related to illumos gate - Feature #7709: hrt2ts and friends are too clever for their own goodClosedPatrick Mooney2016-12-30

Actions
Actions #1

Updated by Patrick Mooney over 1 year ago

  • Related to Feature #7709: hrt2ts and friends are too clever for their own good added
Actions #2

Updated by shua ll 7 months ago

  • Assignee set to shua ll
Actions #3

Updated by shua ll 7 months ago

wrote test program similar to those in previous ticket, but copying conversion logic from uniqtime. I originally tried copying the entire uniqtime function but had issues with tracking down some function calls used which I think are only available in kernel, and specialized for certain archs (specifically gethrestime). That code was not used in the relevant bits tested, so I chose to just focus the test on the conversion code which I intend to change:

#define _FAKE_KERNEL
#include <stdio.h>
#include <sys/kmem.h>
#include <sys/time.h>
#include <sys/systm.h>
#include <sys/sysmacros.h>

#define TESTCOUNT 10000000

void
conv_new(timestruc_t ts, struct timeval *tv)
{
    time_t sec;
    int usec, nsec;

    usec = NSEC2USEC(ts.tv_nsec);
    sec = ts.tv_sec;

    tv->tv_sec = sec;
    tv->tv_usec = usec;
}

void
conv_old(timestruc_t ts, struct timeval *tv)
{
    time_t sec;
    int usec, nsec;

    /*
     * Fast algorithm to convert nsec to usec -- see hrt2ts()
     * in common/os/timers.c for a full description.
     */
    nsec = ts.tv_nsec;
    usec = nsec + (nsec >> 2);
    usec = nsec + (usec >> 1);
    usec = nsec + (usec >> 2);
    usec = nsec + (usec >> 4);
    usec = nsec - (usec >> 3);
    usec = nsec + (usec >> 2);
    usec = nsec + (usec >> 3);
    usec = nsec + (usec >> 4);
    usec = nsec + (usec >> 1);
    usec = nsec + (usec >> 6);
    usec = usec >> 10;
    sec = ts.tv_sec;

    tv->tv_sec = sec;
    tv->tv_usec = usec;
}

void
test_conv(void (*f)(timestruc_t, struct timeval *), const char * name) {
    timespec_t start, end;
    uint64_t diff;
    struct timeval tv;
    timestruc_t ts;

    clock_gettime(CLOCK_MONOTONIC, &start);
    ts.tv_sec = start.tv_sec;
    ts.tv_nsec = start.tv_nsec;
    for (int i=0; i < TESTCOUNT; i++) {
        f(ts, &tv);
    }
    clock_gettime(CLOCK_MONOTONIC, &end);
    if (end.tv_nsec < start.tv_nsec) {
        end.tv_sec -= 1;
        end.tv_nsec += NANOSEC;
    }
    diff = end.tv_nsec - start.tv_nsec;
    diff += (end.tv_sec - start.tv_sec) * NANOSEC;
    printf("%s out: %ld %ld\n", name, tv.tv_sec, tv.tv_usec);
    printf("%s: %lld for %d iterations (%lldns per)\n", name, diff, TESTCOUNT, diff/TESTCOUNT);
}

int 
main(int argc, char** argv) {
    test_conv(conv_old, "uniqtime_conv_old");
    test_conv(conv_new, "uniqtime_conv_new");
}

with following results run inside omnios qemu guest on an i7

host % qemu-system-x86_64 -cpu host -enable-kvm -m 2048 \
    -hda omnios.qcow2 \
    -nic user,model=e1000 \
    -boot d -smp 2
omnios % uname -a
SunOS omnios 5.11 omnios-master-cc133161da i86pc i386 i86pc
omnios % gcc -O2 -m64 -o test_64 test_conv.c && ./test_64
uniqtime_conv_old out: 58584 384109
uniqtime_conv_old: 34933684 for 10000000 iterations (3ns per)
uniqtime_conv_new out: 58584 419166
uniqtime_conv_new: 18413529 for 10000000 iterations (1ns per)
omnios % gcc -O2 -m32 -o test_32 test_conv.c && ./test_32
uniqtime_conv_old out: 58591 231829
uniqtime_conv_old: 55962797 for 10000000 iterations (5ns per)
uniqtime_conv_new out: 58591 287918
uniqtime_conv_new: 29201299 for 10000000 iterations (2ns per)

looks similar to previous tests in #7709 , but test+results added for sanity checking

Actions #4

Updated by Electric Monk 7 months ago

  • Gerrit CR set to 1456
Actions #5

Updated by Toomas Soome 7 months ago

shua ll wrote in #note-3:

with following results run inside omnios qemu guest on an i7
[...]

tsoome@openindiana:~$ gcc -O2 -m64 -o test_64 uniqtime.c && ./test_64
uniqtime.c: In function 'test_conv':
uniqtime.c:58:5: warning: implicit declaration of function 'clock_gettime' [-Wimplicit-function-declaration]
     clock_gettime(CLOCK_MONOTONIC, &start);
     ^~~~~~~~~~~~~
uniqtime_conv_old out: 11939965 851099
uniqtime_conv_old: 201545879 for 10000000 iterations (20ns per)
uniqtime_conv_new out: 11939966 52851
uniqtime_conv_new: 278173248 for 10000000 iterations (27ns per)
tsoome@openindiana:~$ gcc -O2 -m32 -o test_32 uniqtime.c && ./test_32
uniqtime.c: In function 'test_conv':
uniqtime.c:58:5: warning: implicit declaration of function 'clock_gettime' [-Wimplicit-function-declaration]
     clock_gettime(CLOCK_MONOTONIC, &start);
     ^~~~~~~~~~~~~
uniqtime_conv_old out: 11939983 91287
uniqtime_conv_old: 257926684 for 10000000 iterations (25ns per)
uniqtime_conv_new out: 11939983 349546
uniqtime_conv_new: 228006663 for 10000000 iterations (22ns per)
tsoome@openindiana:~$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/gcc/7/lib/gcc/sparc-sun-solaris2.11/7.5.0/lto-wrapper
Target: sparc-sun-solaris2.11
Configured with: /code/oi-userland/components/developer/gcc-7/gcc-gcc-7.5.0-oi-0/configure CC=/usr/gcc/7/bin/gcc CXX=/usr/gcc/7/bin/g++ F77=/usr/gcc/7/bin/gfortran FC=/usr/gcc/7/bin/gfortran CFLAGS=-O2 CXXFLAGS=-O2 FFLAGS='  -O3 -mno-app-regs' FCFLAGS=-O2 LDFLAGS=-m32 PKG_CONFIG_PATH=/usr/lib/pkgconfig --prefix=/usr/gcc/7 --mandir=/usr/gcc/7/share/man --bindir=/usr/gcc/7/bin --libdir=/usr/gcc/7/lib --sbindir=/usr/gcc/7/sbin --sbindir=/usr/gcc/7/bin --libdir=/usr/gcc/7/lib --libexecdir=/usr/gcc/7/lib --host sparc-sun-solaris2.11 --build sparc-sun-solaris2.11 --target sparc-sun-solaris2.11 --with-pkgversion='OpenIndiana 7.5.0-il-0' --with-bugurl=https://bugs.openindiana.org --without-gnu-ld --with-ld=/usr/bin/ld --with-build-time-tools=/usr/gnu/sparc-sun-solaris2.11/bin --without-gnu-as --with-as=/usr/bin/as LDFLAGS=-R/usr/gcc/7/lib --enable-plugins --enable-objc-gc --enable-languages=c,c++,fortran,lto,objc --disable-libitm enable_frame_pointer=yes
Thread model: posix
gcc version 7.5.0 (OpenIndiana 7.5.0-il-0) 
tsoome@openindiana:~$ uname -a
SunOS openindiana 5.11 illumos-6dcbfae4aa sun4v sparc sun4v
tsoome@openindiana:~$ 
Actions

Also available in: Atom PDF