[Bug libstdc++/59177] New: steady_clock::now() and system_clock::now do not use the vdso (and are therefore very slow)

[Bug libstdc++/59177] New: steady_clock::now() and system_clock::now do not use the vdso (and are therefore very slow)

[luto at mit dot edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59177

            Bug ID: 59177
           Summary: steady_clock::now() and system_clock::now do not use
                    the vdso (and are therefore very slow)
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: luto at mit dot edu

std::chrono::steady_clock::now() does this:

#ifdef _GLIBCXX_USE_CLOCK_GETTIME_SYSCALL
      syscall(SYS_clock_gettime, CLOCK_MONOTONIC, &tp);
#else
      clock_gettime(CLOCK_MONOTONIC, &tp);
#endif

I'm not sure what the intent of this condition is, but the effect is that
glibc's clock_gettime (which is very carefully optimized and avoids using
syscalls) is ignored in favor of using syscall(2) (which is very slow).

This appears to have been introduced by this commit:

http://gcc.gnu.org/ml/libstdc++/2013-05/txtfX2KusGj9C.txt

It's a serious slowdown:

steady_clock::now(): 0.0933114µs per iteration
clock_gettime: 0.0230129µs per iteration

as measured by:

#include <chrono>
#include <iostream>
#include <time.h>
using namespace std;

constexpr int iters = 10000;
typedef chrono::duration<double> dsecs;

int main()
{
  auto start = chrono::steady_clock::now();
  for (int i = 0; i < iters; i++)
    chrono::steady_clock::now();
  auto end = chrono::steady_clock::now();

  std::cout << "steady_clock::now(): " << 1e6 *
chrono::duration_cast<dsecs>(end-start).count() / iters << "µs per
iteration\n";

  start = chrono::steady_clock::now();
  timespec ts;
  for (int i = 0; i < iters; i++)
    clock_gettime(CLOCK_MONOTONIC, &ts);
  end = chrono::steady_clock::now();

  std::cout << "clock_gettime: " << 1e6 *
chrono::duration_cast<dsecs>(end-start).count() / iters << "µs per
iteration\n";
}

system_clock appears to behave identically.
>From gcc-bugs-return-434923-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Mon Nov 18 18:38:32 2013
Return-Path: <gcc-bugs-return-434923-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 12693 invoked by alias); 18 Nov 2013 18:38:32 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 12664 invoked by uid 48); 18 Nov 2013 18:38:29 -0000
From: "dominiq at lps dot ens.fr" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug testsuite/59160] The test c-c++-common/cilk-plus/PS/reduction-3.c fails on x86_64-apple-darwin1(0|3)*.
Date: Mon, 18 Nov 2013 18:38:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: testsuite
X-Bugzilla-Version: 4.9.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: dominiq at lps dot ens.fr
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-59160-4-gnjRCj3sYR@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-59160-4@http.gcc.gnu.org/bugzilla/>
References: <bug-59160-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2013-11/txt/msg01700.txt.bz2
Content-length: 202

http://gcc.gnu.org/bugzilla/show_bug.cgi?idY160

--- Comment #10 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
Patch submitted at http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02098.html.

[Bug libstdc++/59177] steady_clock::now() and system_clock::now do not use the vdso (and are therefore very slow)

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59177

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Paolo Carlini <paolo.carlini at oracle dot com> ---
CC-ing Jakub, then.

[Bug libstdc++/59177] steady_clock::now() and system_clock::now do not use the vdso (and are therefore very slow)

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59177

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Just install glibc 2.17 or later if you care about the speed, otherwise it is
fixing much more important problem that the clocks had ABI incompatible
settings depending on common libstdc++ configure options.  If you do have glibc
2.17 or later, clock_gettime(CLOCK_MONOTONIC, &tp); should be used (if not,
that would be a bug somewhere), but earlier glibcs have clock_gettime defined
only in librt, which is undesirable to require.

[Bug libstdc++/59177] steady_clock::now() and system_clock::now do not use the vdso (and are therefore very slow)

[luto at mit dot edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59177

--- Comment #3 from Andy Lutomirski <luto at mit dot edu> ---
I can't get gcc trunk to build right now, but I just distcleaned and rebuilt
the 4.8 branch truck on Fedora 19, which has glibc-2.17-19.fc19.x86_64.  It
defines _GLIBCXX_USE_CLOCK_GETTIME_SYSCALL.  This happens because:

configure:19349: checking for clock_gettime, nanosleep and sched_yield
configure:19378: result: no

I think that the underlying problem is that --enable-libstdcxx-time defaults to
"no".  Shouldn't it default to "yes" (and hence run the fancy configure
checks)?  Configuring with ./configure --enable-libstdcxx-time does the right
thing.

[Bug libstdc++/59177] steady_clock::now() and system_clock::now do not use the vdso (and are therefore very slow)

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59177

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The default for --enable-libstdcxx-time is =auto, which should be the right
thing:
      gnu* | linux* | kfreebsd*-gnu | knetbsd*-gnu)
        AC_MSG_CHECKING([for at least GNU libc 2.17])
        AC_TRY_COMPILE(
          [#include <features.h>],
          [
          #if ! __GLIBC_PREREQ(2, 17)
          #error
          #endif
          ],
          [glibcxx_glibc217=yes], [glibcxx_glibc217=no])
        AC_MSG_RESULT($glibcxx_glibc217)

        if test x"$glibcxx_glibc217" = x"yes"; then
          ac_has_clock_monotonic=yes
          ac_has_clock_realtime=yes
        fi
        ac_has_nanosleep=yes
        ac_has_sched_yield=yes


My trunk build on Fedora 19 today has:
/* Defined if clock_gettime syscall has monotonic and realtime clock support.
   */
/* #undef _GLIBCXX_USE_CLOCK_GETTIME_SYSCALL */

/* Defined if clock_gettime has monotonic clock support. */
#define _GLIBCXX_USE_CLOCK_MONOTONIC 1

/* Defined if clock_gettime has realtime clock support. */
#define _GLIBCXX_USE_CLOCK_REALTIME 1
and there are two clock_gettime calls in libstdc++.so.6 as they should be.

[Bug libstdc++/59177] steady_clock::now() and system_clock::now do not use the vdso (and are therefore very slow)

[luto at mit dot edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59177

Andy Lutomirski <luto at mit dot edu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|4.9.0                       |4.8.3

--- Comment #5 from Andy Lutomirski <luto at mit dot edu> ---
Aha.  I think the issue is only in 4.8.  libstdc++-v3/configure.ac says:

# For clock_gettime, nanosleep and sched_yield support.
# NB: The default is [no], because otherwise it requires linking.
GLIBCXX_ENABLE_LIBSTDCXX_TIME([no])

Aside from the typo in the comment (the end of the sentence seems to have
disappeared), I'm still confused.  Even in the 4.8 branch,
enable_libstdcxx_time=yes doesn't appear to link in librt; only
enable_libstdcxx_time=rt seems to have that effect.

(Please accept my apologies if I'm just completely confused about how autoconf
works here.)

[Bug libstdc++/59177] steady_clock::now() and system_clock::now do not use the vdso (and are therefore very slow)

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59177

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Sure, for GCC 4.8.1 we've only changed minimum things that were required for
correct operation and ABI compatibility, it didn't receive the extra changes
for =auto.

[Bug libstdc++/59177] steady_clock::now() and system_clock::now do not use the vdso (and are therefore very slow)

[luto at mit dot edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59177

--- Comment #7 from Andy Lutomirski <luto at mit dot edu> ---
I guess what I'm saying is: what's wrong with "yes" in 4.8?  From looking at
the code, it still seems like it does the right thing (i.e. not using
clock_gettime if it's not in [posix4]).

[Bug libstdc++/59177] steady_clock::now() and system_clock::now do not use the vdso (and are therefore very slow)

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59177

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
For hosted linux --enable-libstdcxx-time can work just fine, just use it.
But, such a change isn't desirable for the branch, e.g. because the fancy
checks require link tests which aren't usable for some bare metal
configurations, might not be the right thing for Solaris or whatever else, etc.
=auto has been introduced exactly to get some reasonably safe defaults
automatically by default.  So, if you want to change anything on the branch,
supposedly it would be partial backport of the =auto stuff, but only limit it
to the most commonly tested targets or something.  Still, I'd say it is better
to just tell people who want it to --enable-libstdcxx-time themselves if they
want.
In the past it has been ABI incompatible option, now hopefully is not.

[Bug libstdc++/59177] steady_clock::now() and system_clock::now do not use the vdso (and are therefore very slow)

[luto at mit dot edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59177

--- Comment #9 from Andy Lutomirski <luto at mit dot edu> ---
Given that this is C++11-only, it's already fixed on trunk, it's only a
performance issue (as opposed to correctness), and it's more complicated than
just changing the default, I won't argue for a backport.

>From my perspective, feel free to close this.

[Bug libstdc++/59177] steady_clock::now() and system_clock::now do not use the vdso (and are therefore very slow)

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59177

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |WORKSFORME

--- Comment #10 from Paolo Carlini <paolo.carlini at oracle dot com> ---
Ok, thanks.

[Bug libstdc++/59177] steady_clock::now() and system_clock::now do not use the vdso (and are therefore very slow)

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59177

--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Andy Lutomirski from comment #5)
> # For clock_gettime, nanosleep and sched_yield support.
> # NB: The default is [no], because otherwise it requires linking.
> GLIBCXX_ENABLE_LIBSTDCXX_TIME([no])
> 
> Aside from the typo in the comment (the end of the sentence seems to have
> disappeared),

No, the sentence is complete. Comment 8 explains that we don't want to do tests
that require running the linker for the default configuration, because that's
not possible for bare metal targets.

[Bug libstdc++/59177] steady_clock::now() and system_clock::now do not use the vdso (and are therefore very slow)

[luto at mit dot edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59177

--- Comment #12 from Andy Lutomirski <luto at mit dot edu> ---
D'oh!  I assumed that "it" was the resulting library (and thus that it would
require linking *against librt*), not that "it" was the configure test.