Project

General

Profile

Bug #19743

GeneratedEventTimestamp_plugin.cc broken in clang.

Added by Herbert Greenlee over 1 year ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Start date:
04/17/2018
Due date:
% Done:

100%

Estimated time:
Spent time:
Duration:

Description

GeneratedEventTimestamp_plugin.cc gives a different and incorrect time in clang as compared to gcc.

I made a stand alone test program test.cxx (attached) containing the essential code copied from GeneratedEventTimestamp_plugin.cc (nutools v2_21_01)

$ clang++ test.cxx
(many warnings)
$ a.out
110126021417039
$ g++ test.cxx
$ a.out
1523999905013768532

Timestamp in clang is too small by factor or 10^4. This is consistent with behavior observed in the wild using the art framework EmptyEvent source module.

test.cxx (4.21 KB) test.cxx Herbert Greenlee, 04/17/2018 04:28 PM
time_stamp.txt (2.72 KB) time_stamp.txt Herbert Greenlee, 04/19/2018 01:43 PM

Associated revisions

Revision e944996f (diff)
Added by Gianluca Petrillo over 1 year ago

GeneratedEventTimestamp now returns seconds from the Epoch

This solves issue #19743.

Revision 33863c38
Added by Gianluca Petrillo over 1 year ago

GeneratedEventTimestamp now returns seconds from the Epoch

This solves issue #19743.

Revision d7e6366c (diff)
Added by Gianluca Petrillo over 1 year ago

GeneratedEventTimestamp now returns seconds from the Epoch

This solves issue #19743.

Revision e944996f (diff)
Added by Gianluca Petrillo over 1 year ago

GeneratedEventTimestamp now returns seconds from the Epoch

This solves issue #19743.

Revision 7dcad647 (diff)
Added by Gianluca Petrillo over 1 year ago

GeneratedEventTimestamp now returns seconds from the Epoch

This solves issue #19743.

Revision e944996f (diff)
Added by Gianluca Petrillo over 1 year ago

GeneratedEventTimestamp now returns seconds from the Epoch

This solves issue #19743.

Revision 0c036584 (diff)
Added by Lynn Garren over 1 year ago

nutools v2_21_03 with fix for #19743.

History

#1 Updated by Gianluca Petrillo over 1 year ago

  • Status changed from New to Assigned
  • Assignee set to Gianluca Petrillo

#2 Updated by Lynn Garren over 1 year ago

One must, of course, use --std=c++17 when compiling with clang v5_0_1 and --std=c++14 when compiling with gcc v6_4_0. Using these gets rid of the warnings. Also, this code has not been changed in some time.

$ clang++ --std=c++17 -o clangtest test.cxx 
$ ./clangtest 
8072336450453934

$ g++ --std=c++14 -o gcctest test.cxx 
$ ./gcctest 
1524010889101558187

#3 Updated by Lynn Garren over 1 year ago

Here is a sample of a failure message:

%MSG-s ArtException:  PostEndJob 18-Apr-2018 11:26:17 CDT ModuleEndJob
cet::exception caught in art
---- OtherArt BEGIN
  ^[[93mTimeStampDecoder: I do not know how to convert this timestamp: 8130267357464517^[[00m
---- OtherArt END
%MSG

#4 Updated by Kyle Knoepfel over 1 year ago

The time_since_epoch() function returns an implementation-defined duration--i.e. the definition of the epoch does not necessarily correspond to January 1, 1970. See this StackOverflow response by Howard Hinnant, who along with Walter Brown and Marc Paterno designed the C++ <chrono> library:

https://stackoverflow.com/a/29800557/3585575

#5 Updated by Gianluca Petrillo over 1 year ago

  • Status changed from Assigned to Feedback

The art::TimeStamp does not prescribe a specific interpretation of its value, nor LArSoft endorses any.
The intention of this plugin was to provide some source of randomness, which both GCC and Clang in their own ways do. From this point of view, the feature is not broken.

This said, it may be that you are using it beyond the way it was designed for, and maybe that extended use can be accommodated.
If you want to, please detail the use case and requirement this plugin fails to fulfill, and we can work out a solution.
If you don't have any follow up, instead, you can close this ticket.

#6 Updated by Herbert Greenlee over 1 year ago

Find attached a text document containing background information about how various time stamps are used in MicroBooNE, larsoft, and art.

#7 Updated by Gianluca Petrillo over 1 year ago

  • Status changed from Feedback to Resolved

The timestamp plugin GeneratedEventTimeStamp has been modified to add an offset to the high resolution clock to bring the timestamp in the vicinity of the number of nanoseconds past from the UNIX epoch (January 1, 1970). This is not the official definition of the timestamp.

The timestamp preserves the high resolution. The timestamp may be off the number of nanoseconds from the epoch, depending on the implementation of the C++ standard library. This error is currently none with GCC 6.4.0 (where the high resolution clock is already based on that definition) and it may be of about 1 µs in Clang 5.0 (which is the resolution of the system clock; the high resolution clock is not relative to the epoch).

Note that formally the code will be guaranteed to conform the definition only with C++20, which is going to require that the system clock be relative to the epoch, which is only de facto true now.

#8 Updated by Gianluca Petrillo over 1 year ago

The fix is uploaded in branch feature/gp_Issue19743 of nutools.
It has been checked that on the same machine, Clang and GCC return timestamps with a similar value.
A "unit test" is provided, which does not really test anything and just runs EmptyEvent source module with the plugin.
An analyzer module could be written to check that the timestamp is not too far from the expected value — I did not do that though.

#9 Updated by Gianluca Petrillo over 1 year ago

  • % Done changed from 0 to 100

#10 Updated by Gianluca Petrillo over 1 year ago

  • Status changed from Resolved to Closed


Also available in: Atom PDF