Expose $262.agent.monotonicNow() for Test262 use in testing Atomic operation timeouts

RESOLVED FIXED in Firefox 62

Status

()

P3
normal
RESOLVED FIXED
8 months ago
6 months ago

People

(Reporter: rwaldron, Assigned: rwaldron)

Tracking

unspecified
mozilla62
Points:
---

Firefox Tracking Flags

(firefox62 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

8 months ago
In order to safely and accurately test Atomics.wait timeouts, Test262 is extending the $262.agent API to include a monotonicNow() function that's defined as: 


> a function that returns a value that conforms to DOMHighResTimeStamp and is produced in such a way that its semantics conform to Monotonic Clock.

Where DOMHighResTimeStamp and Monotonic Clock are normatively defined in [High Resolution Time Level 2](https://www.w3.org/TR/hr-time-2/).


Relevant: 

- Test262: Defines $262.agent.monotonicNow(), https://github.com/tc39/test262/pull/1535
- JSC: Expose "$262.agent.monotonicNow()" for use in testing Atomic operation timeouts, https://bugs.webkit.org/show_bug.cgi?id=185043
- V8: Expose "$262.agent.monotonicNow()" for use in testing Atomic operation timeouts, https://chromium-review.googlesource.com/c/v8/v8/+/1033426


(patch to follow)
(Assignee)

Updated

8 months ago
Assignee: nobody → waldron.rick
The shell has a dateNow() function that returns a very precise number of milliseconds, but I'm not 100% certain that it's monotonic.  I *think* it is, but I can't find clear guarantees of such in comments.

Browser and shell both have Date.now(), but that uses largely the same mechanism.  (Plus some jitter-safeguarding that might well give us monotonicity, but that won't necessarily always be enabled.)

Browser might also have performance.now() that may have a clear monotonicity guarantee, but I think the resolution of that got destroyed recently, so using it for this is not entirely ideal.

The fix here is likely more or less simple.  But a little research into possibilities is necessary to do this justice.  (And maybe, introducing a shell builtin that *specifically* gives a monotonic high-res counter for best testing, even if in-browser mocking can't use something so fine-grained.)
(In reply to Jeff Walden [:Waldo] from comment #1)
> The shell has a dateNow() function that returns a very precise number of
> milliseconds, but I'm not 100% certain that it's monotonic.  I *think* it
> is, but I can't find clear guarantees of such in comments.

It seems it calls |gettimeofday()| (for XP_UNIX) which this source <http://man7.org/linux/man-pages/man2/gettimeofday.2.html> describes as non-monotonic.

Call hierarchy:

-> https://searchfox.org/mozilla-central/rev/68fdb6cf4f40bea1a1f6c07531ebf58fb8ab060b/js/src/shell/js.cpp#6706
 -> https://searchfox.org/mozilla-central/rev/68fdb6cf4f40bea1a1f6c07531ebf58fb8ab060b/js/src/shell/js.cpp#2333
  -> https://searchfox.org/mozilla-central/rev/68fdb6cf4f40bea1a1f6c07531ebf58fb8ab060b/js/src/vm/Time.cpp#48
   -> https://searchfox.org/mozilla-central/rev/68fdb6cf4f40bea1a1f6c07531ebf58fb8ab060b/js/src/vm/Time.cpp#53
(Assignee)

Comment 3

8 months ago
(In reply to Jeff Walden [:Waldo] from comment #1)
> The fix here is likely more or less simple.  But a little research into
> possibilities is necessary to do this justice.  (And maybe, introducing a
> shell builtin that *specifically* gives a monotonic high-res counter for
> best testing, even if in-browser mocking can't use something so
> fine-grained.)

This was my intuition as well, and is essentially the same solution that I created for JSC https://bugs.webkit.org/show_bug.cgi?id=185043 

Thank you both for the useful details!
Priority: -- → P3
(Assignee)

Comment 4

8 months ago
Created attachment 8972608 [details] [diff] [review]
0001-Bug-1457560-Expose-262.agent.monotonicNow-for-Test26.patch
Attachment #8972608 - Flags: review?(jwalden+bmo)
Attachment #8972608 - Flags: review?(jorendorff)
Comment on attachment 8972608 [details] [diff] [review]
0001-Bug-1457560-Expose-262.agent.monotonicNow-for-Test26.patch

Review of attachment 8972608 [details] [diff] [review]:
-----------------------------------------------------------------

Looks like exactly what we need; r=me on the idea, and leaving r?Waldo for a verdict on whether we can use <chrono> and std:steady_clock.
Attachment #8972608 - Flags: review?(jorendorff) → review+
Comment on attachment 8972608 [details] [diff] [review]
0001-Bug-1457560-Expose-262.agent.monotonicNow-for-Test26.patch

Review of attachment 8972608 [details] [diff] [review]:
-----------------------------------------------------------------

SpiderMonkey already uses some <chrono> stuff, so this doesn't put us on a road to perdition that we aren't already on.  :-)

Feel free to post a new patch here with the requested minimal tweaks, and I'll slurp it into my tree for landing the next time I have stuff to land.

::: js/src/builtin/TestingFunctions.cpp
@@ +4951,5 @@
>  
> +static bool
> +MonotonicNow(JSContext* cx, unsigned argc, Value* vp)
> +{
> +    using namespace std::chrono;

I'd prefer

  using std::chrono::duration_cast;
  using std::chrono::nanoseconds;
  using std::chrono::steady_clock;

to pick out only what we use.

@@ +4955,5 @@
> +    using namespace std::chrono;
> +
> +    CallArgs args = CallArgsFromVp(argc, vp);
> +
> +    double now = duration_cast<milliseconds>(steady_clock::now().time_since_epoch()).count();

Why milliseconds, if you can use any duration here, and greater precision allows stricter testing of implementation behavior?  Seems like we should use the most precise time available.  So I think this should be (with a justifying comment):

    uint64_t now = duration_cast<nanoseconds>(steady_clock::now().time_since_epoch()).count();

    // We ubiquitously presume double is IEEE-754 double precision, so any
    // uint64_t fits in double's value range and this conversion must succeed.
    // If the conversion is inexact, C++11 [conv.fpint]p2 says an
    // implementation-defined choice of value is computed.  *Technically* that
    // doesn't guarantee nanosecond-precision counts will round monotonically,
    // but practically we probably can make that assumption, so we do so here.
    args.rval().setNumber(static_cast<double>(now));
    return true;

@@ +4958,5 @@
> +
> +    double now = duration_cast<milliseconds>(steady_clock::now().time_since_epoch()).count();
> +    args.rval().setNumber(now);
> +
> +    return true;

I'd say remove the blank line above -- we usually consider set-rval-and-return as one coherent operation that we keep together, often separate from the rest of the function body.

::: js/src/tests/test262-host.js
@@ +131,5 @@
> +        leaving() {},
> +
> +        monotonicNow() {
> +            return monotonicNow();
> +        }

Tack a comma on after this.  (Should have been one after leaving's body before, really.)

@@ +196,5 @@
>                  },
> +
> +                monotonicNow() {
> +                    return monotonicNow();
> +                }

Tack on a trailing comma here too (note this *is* consistent with how sleep just above had one).
Attachment #8972608 - Flags: review?(jwalden+bmo) → review+
(Assignee)

Comment 7

7 months ago
(In reply to Jeff Walden [:Waldo] from comment #6)
> > +    double now = duration_cast<milliseconds>(steady_clock::now().time_since_epoch()).count();
> 
> Why milliseconds, if you can use any duration here, and greater precision
> allows stricter testing of implementation behavior?  Seems like we should
> use the most precise time available.  So I think this should be (with a
> justifying comment):
> 

The "timeout" argument in Atomics.wait(i32a, index, value, timeout) is a millisecond value and presently, Test262 uses Date.now() to measure Atomics.wait() timeouts. monotonicNow() is intended to be a replacement for occurrences of Date.now() to prevent measurements of Atomics.wait(...) timeouts from being affected by system ("time of day") clock behavior (time travel, etc). Specifically: Test262 needs to be able to test for spurious wake ups by measuring time elapsed from before an Atomics.wait() call and after. Filip Pizlo, Leo Balter and I were reviewing Atomics.wait() tests and we agreed that Test262 needed to grow a $262.agent.monotonicNow() and that it will align with the DOMHighResTimeStamp (aka. performance.now()) specification. 

Thanks again for the review! I will update the patch shortly.
(Assignee)

Comment 8

7 months ago
Created attachment 8974769 [details] [diff] [review]
0001-Bug-1457560-Expose-262.agent.monotonicNow-for-Test26.patch

Updates per review
Attachment #8972608 - Attachment is obsolete: true
Attachment #8974769 - Flags: review?(jwalden+bmo)
Comment on attachment 8974769 [details] [diff] [review]
0001-Bug-1457560-Expose-262.agent.monotonicNow-for-Test26.patch

Review of attachment 8974769 [details] [diff] [review]:
-----------------------------------------------------------------

I have enqueued this for pushing.  Thanks!  Too bad about dependence/requirement of a particular timestamp resolution here, since I thought this was supposed to be usable for gauging non-raciness sorts of requirements, where more precision is better.
Attachment #8974769 - Flags: review?(jwalden+bmo) → review+
I don't plan to push this immediately, only when I have a few other things to push (and I just cleared that queue the other day).  But if you want me to push faster, I certainly can -- drop a note if that's desirable.
(Assignee)

Comment 11

7 months ago
(In reply to Jeff Walden [:Waldo] from comment #10)
> I don't plan to push this immediately, only when I have a few other things
> to push (and I just cleared that queue the other day).  But if you want me
> to push faster, I certainly can -- drop a note if that's desirable.

Do you think that might be sometime in the next week? If so, it can wait for that. 

Thanks again!

Comment 12

7 months ago
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4fd5393d398
Expose $262.agent.monotonicNow() for Test262 use in testing Atomic operation timeouts.  r=jwalden
Hrm.  Certain builds on treeherder are complaining about

  TEST-UNEXPECTED-FAIL | check_stdcxx | We do not want these libstdc++ symbol versions to be used:

referring to some of the new chrono symbols used here, and so I backed out the patch.  Not sure what's up, precisely.  However, https://searchfox.org/mozilla-central/source/toolkit/components/telemetry/Telemetry.cpp#1693 does not look too promising as to the usability of these symbols.  :-(  froyndj, you know what's up here, probably?
Flags: needinfo?(nfroyd)
Relevant portions of a build log, for posterity:

[task 2018-05-12T04:07:20.776Z] rm -f libjs_static.a
[task 2018-05-12T04:07:20.777Z] libmozjs-62a1.so
[task 2018-05-12T04:07:20.777Z] rm -f libmozjs-62a1.so
[task 2018-05-12T04:07:20.786Z] ar crs libjs_static.a  ../ittnotify_static.o ../jitprofiling.o ../Array.o ../RegExp.o ../BinSource-auto.o ../BinSource.o ../BinToken.o ../BinTokenReaderBase.o ../BinTokenReaderMultipart.o ../BinTokenReaderTester.o ../Parser.o ../StoreBuffer.o ../Disassembler-x86-shared.o ../jsmath.o ../jsutil.o ../pm_linux.o ../DoubleToString.o ../Interpreter.o ../JSAtom.o ../ProfilingStack.o ../TraceLogging.o ../TraceLoggingGraph.o ../TraceLoggingTypes.o ../VTuneWrapper.o ../Unified_cpp_js_src0.o ../Unified_cpp_js_src1.o ../Unified_cpp_js_src10.o ../Unified_cpp_js_src11.o ../Unified_cpp_js_src12.o ../Unified_cpp_js_src13.o ../Unified_cpp_js_src14.o ../Unified_cpp_js_src15.o ../Unified_cpp_js_src16.o ../Unified_cpp_js_src17.o ../Unified_cpp_js_src18.o ../Unified_cpp_js_src19.o ../Unified_cpp_js_src2.o ../Unified_cpp_js_src20.o ../Unified_cpp_js_src21.o ../Unified_cpp_js_src22.o ../Unified_cpp_js_src23.o ../Unified_cpp_js_src24.o ../Unified_cpp_js_src25.o ../Unified_cpp_js_src26.o ../Unified_cpp_js_src27.o ../Unified_cpp_js_src28.o ../Unified_cpp_js_src29.o ../Unified_cpp_js_src3.o ../Unified_cpp_js_src30.o ../Unified_cpp_js_src31.o ../Unified_cpp_js_src32.o ../Unified_cpp_js_src33.o ../Unified_cpp_js_src34.o ../Unified_cpp_js_src35.o ../Unified_cpp_js_src36.o ../Unified_cpp_js_src37.o ../Unified_cpp_js_src38.o ../Unified_cpp_js_src39.o ../Unified_cpp_js_src4.o ../Unified_cpp_js_src40.o ../Unified_cpp_js_src41.o ../Unified_cpp_js_src42.o ../Unified_cpp_js_src43.o ../Unified_cpp_js_src44.o ../Unified_cpp_js_src5.o ../Unified_cpp_js_src6.o ../Unified_cpp_js_src7.o ../Unified_cpp_js_src8.o ../Unified_cpp_js_src9.o ../../../modules/fdlibm/src/e_acos.o ../../../modules/fdlibm/src/e_acosh.o ../../../modules/fdlibm/src/e_asin.o ../../../modules/fdlibm/src/e_atan2.o ../../../modules/fdlibm/src/e_atanh.o ../../../modules/fdlibm/src/e_cosh.o ../../../modules/fdlibm/src/e_exp.o ../../../modules/fdlibm/src/e_hypot.o ../../../modules/fdlibm/src/e_log.o ../../../modules/fdlibm/src/e_log10.o ../../../modules/fdlibm/src/e_log2.o ../../../modules/fdlibm/src/e_pow.o ../../../modules/fdlibm/src/e_sinh.o ../../../modules/fdlibm/src/e_sqrt.o ../../../modules/fdlibm/src/k_exp.o ../../../modules/fdlibm/src/s_asinh.o ../../../modules/fdlibm/src/s_atan.o ../../../modules/fdlibm/src/s_cbrt.o ../../../modules/fdlibm/src/s_ceil.o ../../../modules/fdlibm/src/s_ceilf.o ../../../modules/fdlibm/src/s_copysign.o ../../../modules/fdlibm/src/s_expm1.o ../../../modules/fdlibm/src/s_fabs.o ../../../modules/fdlibm/src/s_floor.o ../../../modules/fdlibm/src/s_floorf.o ../../../modules/fdlibm/src/s_log1p.o ../../../modules/fdlibm/src/s_nearbyint.o ../../../modules/fdlibm/src/s_rint.o ../../../modules/fdlibm/src/s_rintf.o ../../../modules/fdlibm/src/s_scalbn.o ../../../modules/fdlibm/src/s_tanh.o ../../../modules/fdlibm/src/s_trunc.o ../../../modules/fdlibm/src/s_truncf.o ../../../config/external/icu/common/appendable.o ../../../config/external/icu/common/bmpset.o ../../../config/external/icu/common/brkeng.o ../../../config/external/icu/common/brkiter.o ../../../config/external/icu/common/bytesinkutil.o ../../../config/external/icu/common/bytestream.o ../../../config/external/icu/common/bytestrie.o ../../../config/external/icu/common/bytestriebuilder.o ../../../config/external/icu/common/bytestrieiterator.o ../../../config/external/icu/common/caniter.o ../../../config/external/icu/common/chariter.o ../../../config/external/icu/common/charstr.o ../../../config/external/icu/common/cmemory.o ../../../config/external/icu/common/cstr.o ../../../config/external/icu/common/cstring.o ../../../config/external/icu/common/cwchar.o ../../../config/external/icu/common/dictbe.o ../../../config/external/icu/common/dictionarydata.o ../../../config/external/icu/common/dtintrv.o ../../../config/external/icu/common/edits.o ../../../config/external/icu/common/errorcode.o ../../../config/external/icu/common/filteredbrk.o ../../../config/external/icu/common/filterednormalizer2.o ../../../config/external/icu/common/icudataver.o ../../../config/external/icu/common/icuplug.o ../../../config/external/icu/common/listformatter.o ../../../config/external/icu/common/loadednormalizer2impl.o ../../../config/external/icu/common/locavailable.o ../../../config/external/icu/common/locbased.o ../../../config/external/icu/common/locdispnames.o ../../../config/external/icu/common/locdspnm.o ../../../config/external/icu/common/locid.o ../../../config/external/icu/common/loclikely.o ../../../config/external/icu/common/locmap.o ../../../config/external/icu/common/locresdata.o ../../../config/external/icu/common/locutil.o ../../../config/external/icu/common/messagepattern.o ../../../config/external/icu/common/normalizer2.o ../../../config/external/icu/common/normalizer2impl.o ../../../config/external/icu/common/normlzr.o ../../../config/external/icu/common/parsepos.o ../../../config/external/icu/common/patternprops.o ../../../config/external/icu/common/pluralmap.o ../../../config/external/icu/common/propname.o ../../../config/external/icu/common/propsvec.o ../../../config/external/icu/common/punycode.o ../../../config/external/icu/common/putil.o ../../../config/external/icu/common/rbbi.o ../../../config/external/icu/common/rbbi_cache.o ../../../config/external/icu/common/rbbidata.o ../../../config/external/icu/common/rbbinode.o ../../../config/external/icu/common/rbbirb.o ../../../config/external/icu/common/rbbiscan.o ../../../config/external/icu/common/rbbisetb.o ../../../config/external/icu/common/rbbistbl.o ../../../config/external/icu/common/rbbitblb.o ../../../config/external/icu/common/resbund.o ../../../config/external/icu/common/resbund_cnv.o ../../../config/external/icu/common/resource.o ../../../config/external/icu/common/ruleiter.o ../../../config/external/icu/common/schriter.o ../../../config/external/icu/common/serv.o ../../../config/external/icu/common/servlk.o ../../../config/external/icu/common/servlkf.o ../../../config/external/icu/common/servls.o ../../../config/external/icu/common/servnotf.o ../../../config/external/icu/common/servrbf.o ../../../config/external/icu/common/servslkf.o ../../../config/external/icu/common/sharedobject.o ../../../config/external/icu/common/simpleformatter.o ../../../config/external/icu/common/stringpiece.o ../../../config/external/icu/common/stringtriebuilder.o ../../../config/external/icu/common/uarrsort.o ../../../config/external/icu/common/ubidi.o ../../../config/external/icu/common/ubidi_props.o ../../../config/external/icu/common/ubidiln.o ../../../config/external/icu/common/ubiditransform.o ../../../config/external/icu/common/ubidiwrt.o ../../../config/external/icu/common/ubrk.o ../../../config/external/icu/common/ucase.o ../../../config/external/icu/common/ucasemap.o ../../../config/external/icu/common/ucasemap_titlecase_brkiter.o ../../../config/external/icu/common/ucat.o ../../../config/external/icu/common/uchar.o ../../../config/external/icu/common/ucharstrie.o ../../../config/external/icu/common/ucharstriebuilder.o ../../../config/external/icu/common/ucharstrieiterator.o ../../../config/external/icu/common/uchriter.o ../../../config/external/icu/common/ucln_cmn.o ../../../config/external/icu/common/ucmndata.o ../../../config/external/icu/common/ucnv.o ../../../config/external/icu/common/ucnv2022.o ../../../config/external/icu/common/ucnv_bld.o ../../../config/external/icu/common/ucnv_cb.o ../../../config/external/icu/common/ucnv_cnv.o ../../../config/external/icu/common/ucnv_ct.o ../../../config/external/icu/common/ucnv_err.o ../../../config/external/icu/common/ucnv_ext.o ../../../config/external/icu/common/ucnv_io.o ../../../config/external/icu/common/ucnv_lmb.o ../../../config/external/icu/common/ucnv_set.o ../../../config/external/icu/common/ucnv_u16.o ../../../config/external/icu/common/ucnv_u32.o ../../../config/external/icu/common/ucnv_u7.o ../../../config/external/icu/common/ucnv_u8.o ../../../config/external/icu/common/ucnvbocu.o ../../../config/external/icu/common/ucnvdisp.o ../../../config/external/icu/common/ucnvhz.o ../../../config/external/icu/common/ucnvisci.o ../../../config/external/icu/common/ucnvlat1.o ../../../config/external/icu/common/ucnvmbcs.o ../../../config/external/icu/common/ucnvscsu.o ../../../config/external/icu/common/ucnvsel.o ../../../config/external/icu/common/ucol_swp.o ../../../config/external/icu/common/ucurr.o ../../../config/external/icu/common/udata.o ../../../config/external/icu/common/udatamem.o ../../../config/external/icu/common/udataswp.o ../../../config/external/icu/common/uenum.o ../../../config/external/icu/common/uhash.o ../../../config/external/icu/common/uhash_us.o ../../../config/external/icu/common/uidna.o ../../../config/external/icu/common/uinit.o ../../../config/external/icu/common/uinvchar.o ../../../config/external/icu/common/uiter.o ../../../config/external/icu/common/ulist.o ../../../config/external/icu/common/ulistformatter.o ../../../config/external/icu/common/uloc.o ../../../config/external/icu/common/uloc_keytype.o ../../../config/external/icu/common/uloc_tag.o ../../../config/external/icu/common/umapfile.o ../../../config/external/icu/common/umath.o ../../../config/external/icu/common/umutex.o ../../../config/external/icu/common/unames.o ../../../config/external/icu/common/unifiedcache.o ../../../config/external/icu/common/unifilt.o ../../../config/external/icu/common/unifunct.o ../../../config/external/icu/common/uniset.o ../../../config/external/icu/common/uniset_closure.o ../../../config/external/icu/common/uniset_props.o ../../../config/external/icu/common/unisetspan.o ../../../config/external/icu/common/unistr.o ../../../config/external/icu/common/unistr_case.o ../../../config/external/icu/common/unistr_case_locale.o ../../../config/external/icu/common/unistr_cnv.o ../../../config/external/icu/common/unistr_props.o ../../../config/external/icu/common/unistr_titlecase_brkiter.o ../../../config/external/icu/common/unorm.o ../../../config/external/icu/common/unormcmp.o ../../../config/external/icu/common/uobject.o ../../../config/external/icu/common/uprops.o ../../../config/external/icu/common/ures_cnv.o ../../../config/external/icu/common/uresbund.o ../../../config/external/icu/common/uresdata.o ../../../config/external/icu/common/usc_impl.o ../../../config/external/icu/common/uscript.o ../../../config/external/icu/common/uscript_props.o ../../../config/external/icu/common/uset.o ../../../config/external/icu/common/uset_props.o ../../../config/external/icu/common/usetiter.o ../../../config/external/icu/common/ushape.o ../../../config/external/icu/common/usprep.o ../../../config/external/icu/common/ustack.o ../../../config/external/icu/common/ustr_cnv.o ../../../config/external/icu/common/ustr_titlecase_brkiter.o ../../../config/external/icu/common/ustr_wcs.o ../../../config/external/icu/common/ustrcase.o ../../../config/external/icu/common/ustrcase_locale.o ../../../config/external/icu/common/ustrenum.o ../../../config/external/icu/common/ustrfmt.o ../../../config/external/icu/common/ustring.o ../../../config/external/icu/common/ustrtrns.o ../../../config/external/icu/common/utext.o ../../../config/external/icu/common/utf_impl.o ../../../config/external/icu/common/util.o ../../../config/external/icu/common/util_props.o ../../../config/external/icu/common/utrace.o ../../../config/external/icu/common/utrie.o ../../../config/external/icu/common/utrie2.o ../../../config/external/icu/common/utrie2_builder.o ../../../config/external/icu/common/uts46.o ../../../config/external/icu/common/utypes.o ../../../config/external/icu/common/uvector.o ../../../config/external/icu/common/uvectr32.o ../../../config/external/icu/common/uvectr64.o ../../../config/external/icu/common/wintz.o ../../../config/external/icu/i18n/affixpatternparser.o ../../../config/external/icu/i18n/alphaindex.o ../../../config/external/icu/i18n/anytrans.o ../../../config/external/icu/i18n/astro.o ../../../config/external/icu/i18n/basictz.o ../../../config/external/icu/i18n/bocsu.o ../../../config/external/icu/i18n/brktrans.o ../../../config/external/icu/i18n/buddhcal.o ../../../config/external/icu/i18n/calendar.o ../../../config/external/icu/i18n/casetrn.o ../../../config/external/icu/i18n/cecal.o ../../../config/external/icu/i18n/chnsecal.o ../../../config/external/icu/i18n/choicfmt.o ../../../config/external/icu/i18n/coleitr.o ../../../config/external/icu/i18n/coll.o ../../../config/external/icu/i18n/collation.o ../../../config/external/icu/i18n/collationbuilder.o ../../../config/external/icu/i18n/collationcompare.o ../../../config/external/icu/i18n/collationdata.o ../../../config/external/icu/i18n/collationdatabuilder.o ../../../config/external/icu/i18n/collationdatareader.o ../../../config/external/icu/i18n/collationdatawriter.o ../../../config/external/icu/i18n/collationfastlatin.o ../../../config/external/icu/i18n/collationfastlatinbuilder.o ../../../config/external/icu/i18n/collationfcd.o ../../../config/external/icu/i18n/collationiterator.o ../../../config/external/icu/i18n/collationkeys.o ../../../config/external/icu/i18n/collationroot.o ../../../config/external/icu/i18n/collationrootelements.o ../../../config/external/icu/i18n/collationruleparser.o ../../../config/external/icu/i18n/collationsets.o ../../../config/external/icu/i18n/collationsettings.o ../../../config/external/icu/i18n/collationtailoring.o ../../../config/external/icu/i18n/collationweights.o ../../../config/external/icu/i18n/compactdecimalformat.o ../../../config/external/icu/i18n/coptccal.o ../../../config/external/icu/i18n/cpdtrans.o ../../../config/external/icu/i18n/csdetect.o ../../../config/external/icu/i18n/csmatch.o ../../../config/external/icu/i18n/csr2022.o ../../../config/external/icu/i18n/csrecog.o ../../../config/external/icu/i18n/csrmbcs.o ../../../config/external/icu/i18n/csrsbcs.o ../../../config/external/icu/i18n/csrucode.o ../../../config/external/icu/i18n/csrutf8.o ../../../config/external/icu/i18n/curramt.o ../../../config/external/icu/i18n/currfmt.o ../../../config/external/icu/i18n/currpinf.o ../../../config/external/icu/i18n/currunit.o ../../../config/external/icu/i18n/dangical.o ../../../config/external/icu/i18n/datefmt.o ../../../config/external/icu/i18n/dayperiodrules.o ../../../config/external/icu/i18n/dcfmtsym.o ../../../config/external/icu/i18n/decContext.o ../../../config/external/icu/i18n/decNumber.o ../../../config/external/icu/i18n/decfmtst.o ../../../config/external/icu/i18n/decimalformatpattern.o ../../../config/external/icu/i18n/decimfmt.o ../../../config/external/icu/i18n/decimfmtimpl.o ../../../config/external/icu/i18n/digitaffix.o ../../../config/external/icu/i18n/digitaffixesandpadding.o ../../../config/external/icu/i18n/digitformatter.o ../../../config/external/icu/i18n/digitgrouping.o ../../../config/external/icu/i18n/digitinterval.o ../../../config/external/icu/i18n/digitlst.o ../../../config/external/icu/i18n/double-conversion-bignum-dtoa.o ../../../config/external/icu/i18n/double-conversion-bignum.o ../../../config/external/icu/i18n/double-conversion-cached-powers.o ../../../config/external/icu/i18n/double-conversion-diy-fp.o ../../../config/external/icu/i18n/double-conversion-fast-dtoa.o ../../../config/external/icu/i18n/double-conversion.o ../../../config/external/icu/i18n/dtfmtsym.o ../../../config/external/icu/i18n/dtitvfmt.o ../../../config/external/icu/i18n/dtitvinf.o ../../../config/external/icu/i18n/dtptngen.o ../../../config/external/icu/i18n/dtrule.o ../../../config/external/icu/i18n/esctrn.o ../../../config/external/icu/i18n/ethpccal.o ../../../config/external/icu/i18n/fmtable.o ../../../config/external/icu/i18n/fmtable_cnv.o ../../../config/external/icu/i18n/format.o ../../../config/external/icu/i18n/fphdlimp.o ../../../config/external/icu/i18n/fpositer.o ../../../config/external/icu/i18n/funcrepl.o ../../../config/external/icu/i18n/gender.o ../../../config/external/icu/i18n/gregocal.o ../../../config/external/icu/i18n/gregoimp.o ../../../config/external/icu/i18n/hebrwcal.o ../../../config/external/icu/i18n/indiancal.o ../../../config/external/icu/i18n/inputext.o ../../../config/external/icu/i18n/islamcal.o ../../../config/external/icu/i18n/japancal.o ../../../config/external/icu/i18n/measfmt.o ../../../config/external/icu/i18n/measunit.o ../../../config/external/icu/i18n/measure.o ../../../config/external/icu/i18n/msgfmt.o ../../../config/external/icu/i18n/name2uni.o ../../../config/external/icu/i18n/nfrs.o ../../../config/external/icu/i18n/nfrule.o ../../../config/external/icu/i18n/nfsubs.o ../../../config/external/icu/i18n/nortrans.o ../../../config/external/icu/i18n/nounit.o ../../../config/external/icu/i18n/nultrans.o ../../../config/external/icu/i18n/number_affixutils.o ../../../config/external/icu/i18n/number_compact.o ../../../config/external/icu/i18n/number_decimalquantity.o ../../../config/external/icu/i18n/number_decimfmtprops.o ../../../config/external/icu/i18n/number_fluent.o ../../../config/external/icu/i18n/number_formatimpl.o ../../../config/external/icu/i18n/number_grouping.o ../../../config/external/icu/i18n/number_integerwidth.o ../../../config/external/icu/i18n/number_longnames.o ../../../config/external/icu/i18n/number_modifiers.o ../../../config/external/icu/i18n/number_notation.o ../../../config/external/icu/i18n/number_padding.o ../../../config/external/icu/i18n/number_patternmodifier.o ../../../config/external/icu/i18n/number_patternstring.o ../../../config/external/icu/i18n/number_rounding.o ../../../config/external/icu/i18n/number_scientific.o ../../../config/external/icu/i18n/number_stringbuilder.o ../../../config/external/icu/i18n/numfmt.o ../../../config/external/icu/i18n/numsys.o ../../../config/external/icu/i18n/olsontz.o ../../../config/external/icu/i18n/persncal.o ../../../config/external/icu/i18n/pluralaffix.o ../../../config/external/icu/i18n/plurfmt.o ../../../config/external/icu/i18n/plurrule.o ../../../config/external/icu/i18n/precision.o ../../../config/external/icu/i18n/quant.o ../../../config/external/icu/i18n/quantityformatter.o ../../../config/external/icu/i18n/rbnf.o ../../../config/external/icu/i18n/rbt.o ../../../config/external/icu/i18n/rbt_data.o ../../../config/external/icu/i18n/rbt_pars.o ../../../config/external/icu/i18n/rbt_rule.o ../../../config/external/icu/i18n/rbt_set.o ../../../config/external/icu/i18n/rbtz.o ../../../config/external/icu/i18n/regexcmp.o ../../../config/external/icu/i18n/regeximp.o ../../../config/external/icu/i18n/regexst.o ../../../config/external/icu/i18n/regextxt.o ../../../config/external/icu/i18n/region.o ../../../config/external/icu/i18n/reldatefmt.o ../../../config/external/icu/i18n/reldtfmt.o ../../../config/external/icu/i18n/rematch.o ../../../config/external/icu/i18n/remtrans.o ../../../config/external/icu/i18n/repattrn.o ../../../config/external/icu/i18n/rulebasedcollator.o ../../../config/external/icu/i18n/scientificnumberformatter.o ../../../config/external/icu/i18n/scriptset.o ../../../config/external/icu/i18n/search.o ../../../config/external/icu/i18n/selfmt.o ../../../config/external/icu/i18n/sharedbreakiterator.o ../../../config/external/icu/i18n/simpletz.o ../../../config/external/icu/i18n/smallintformatter.o ../../../config/external/icu/i18n/smpdtfmt.o ../../../config/external/icu/i18n/smpdtfst.o ../../../config/external/icu/i18n/sortkey.o ../../../config/external/icu/i18n/standardplural.o ../../../config/external/icu/i18n/strmatch.o ../../../config/external/icu/i18n/strrepl.o ../../../config/external/icu/i18n/stsearch.o ../../../config/external/icu/i18n/taiwncal.o ../../../config/external/icu/i18n/timezone.o ../../../config/external/icu/i18n/titletrn.o ../../../config/external/icu/i18n/tmunit.o ../../../config/external/icu/i18n/tmutamt.o ../../../config/external/icu/i18n/tmutfmt.o ../../../config/external/icu/i18n/tolowtrn.o ../../../config/external/icu/i18n/toupptrn.o ../../../config/external/icu/i18n/translit.o ../../../config/external/icu/i18n/transreg.o ../../../config/external/icu/i18n/tridpars.o ../../../config/external/icu/i18n/tzfmt.o ../../../config/external/icu/i18n/tzgnames.o ../../../config/external/icu/i18n/tznames.o ../../../config/external/icu/i18n/tznames_impl.o ../../../config/external/icu/i18n/tzrule.o ../../../config/external/icu/i18n/tztrans.o ../../../config/external/icu/i18n/ucal.o ../../../config/external/icu/i18n/ucln_in.o ../../../config/external/icu/i18n/ucol.o ../../../config/external/icu/i18n/ucol_res.o ../../../config/external/icu/i18n/ucol_sit.o ../../../config/external/icu/i18n/ucoleitr.o ../../../config/external/icu/i18n/ucsdet.o ../../../config/external/icu/i18n/udat.o ../../../config/external/icu/i18n/udateintervalformat.o ../../../config/external/icu/i18n/udatpg.o ../../../config/external/icu/i18n/ufieldpositer.o ../../../config/external/icu/i18n/uitercollationiterator.o ../../../config/external/icu/i18n/ulocdata.o ../../../config/external/icu/i18n/umsg.o ../../../config/external/icu/i18n/unesctrn.o ../../../config/external/icu/i18n/uni2name.o ../../../config/external/icu/i18n/unum.o ../../../config/external/icu/i18n/unumsys.o ../../../config/external/icu/i18n/upluralrules.o ../../../config/external/icu/i18n/uregex.o ../../../config/external/icu/i18n/uregexc.o ../../../config/external/icu/i18n/uregion.o ../../../config/external/icu/i18n/usearch.o ../../../config/external/icu/i18n/uspoof.o ../../../config/external/icu/i18n/uspoof_build.o ../../../config/external/icu/i18n/uspoof_conf.o ../../../config/external/icu/i18n/uspoof_impl.o ../../../config/external/icu/i18n/utf16collationiterator.o ../../../config/external/icu/i18n/utf8collationiterator.o ../../../config/external/icu/i18n/utmscale.o ../../../config/external/icu/i18n/utrans.o ../../../config/external/icu/i18n/valueformatter.o ../../../config/external/icu/i18n/visibledigits.o ../../../config/external/icu/i18n/vtzone.o ../../../config/external/icu/i18n/vzone.o ../../../config/external/icu/i18n/windtfmt.o ../../../config/external/icu/i18n/winnmfmt.o ../../../config/external/icu/i18n/wintzimpl.o ../../../config/external/icu/i18n/zonemeta.o ../../../config/external/icu/i18n/zrule.o ../../../config/external/icu/i18n/ztrans.o ../../../config/external/icu/data/icudata.o
[task 2018-05-12T04:07:20.787Z] /builds/worker/workspace/gcc/bin/g++ -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wduplicated-cond -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wformat -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -freorder-blocks -O3 -fno-omit-frame-pointer -Werror  -fPIC -shared -Wl,-z,defs -Wl,--gc-sections -Wl,-h,libmozjs-62a1.so -o libmozjs-62a1.so /builds/worker/workspace/build/src/obj-spider/js/src/build/libmozjs-62a1_so.list  -lpthread -Wl,-z,noexecstack -Wl,-z,text -Wl,-z,relro -Wl,--build-id -Wl,-version-script,symverscript -Wl,-rpath-link,/builds/worker/workspace/build/src/obj-spider/dist/bin -Wl,-rpath-link,/builds/worker/workspace/build/src/obj-spider/dist/lib     ../../../config/external/nspr/pr/libnspr4.so ../../../config/external/nspr/libc/libplc4.so ../../../config/external/nspr/ds/libplds4.so   -lm -ldl  -lz -lm -ldl -lrt
[task 2018-05-12T04:07:27.281Z] TEST-UNEXPECTED-FAIL | check_stdcxx | We do not want these libstdc++ symbol versions to be used:
[task 2018-05-12T04:07:27.281Z]   _ZNSt6chrono3_V212steady_clock3nowEv@GLIBCXX_3.4.19
[task 2018-05-12T04:07:27.282Z] /builds/worker/workspace/build/src/config/rules.mk:679: recipe for target 'libmozjs-62a1.so' failed
[task 2018-05-12T04:07:27.282Z] make[3]: *** [libmozjs-62a1.so] Error 1
[task 2018-05-12T04:07:27.282Z] make[3]: *** Deleting file 'libmozjs-62a1.so'
[task 2018-05-12T04:07:27.317Z] make[3]: Leaving directory '/builds/worker/workspace/build/src/obj-spider/js/src/build'
[task 2018-05-12T04:07:27.317Z] /builds/worker/workspace/build/src/config/recurse.mk:73: recipe for target 'js/src/build/target' failed
[task 2018-05-12T04:07:27.317Z] make[2]: *** [js/src/build/target] Error 2
[task 2018-05-12T04:07:27.317Z] make[2]: Leaving directory '/builds/worker/workspace/build/src/obj-spider'
[task 2018-05-12T04:07:27.318Z] /builds/worker/workspace/build/src/config/recurse.mk:32: recipe for target 'compile' failed
[task 2018-05-12T04:07:27.318Z] make[1]: *** [compile] Error 2
[task 2018-05-12T04:07:27.318Z] make[1]: Leaving directory '/builds/worker/workspace/build/src/obj-spider'
[task 2018-05-12T04:07:27.318Z] /builds/worker/workspace/build/src/config/rules.mk:418: recipe for target 'default' failed
[task 2018-05-12T04:07:27.318Z] make: *** [default] Error 2
[task 2018-05-12T04:07:27.318Z] make: Leaving directory '/builds/worker/workspace/build/src/obj-spider'
[task 2018-05-12T04:07:27.318Z] Traceback (most recent call last):
[task 2018-05-12T04:07:27.318Z]   File "/builds/worker/workspace/build/src/js/src/devtools/automation/autospider.py", line 399, in <module>
[task 2018-05-12T04:07:27.318Z]     run_command('%s -w %s' % (MAKE, MAKEFLAGS), shell=True, check=True)
[task 2018-05-12T04:07:27.318Z]   File "/builds/worker/workspace/build/src/js/src/devtools/automation/autospider.py", line 330, in run_command
[task 2018-05-12T04:07:27.318Z]     raise subprocess.CalledProcessError(status, command, output=stderr)
[task 2018-05-12T04:07:27.318Z] subprocess.CalledProcessError: Command 'make -w -j6' returned non-zero exit status 2
[task 2018-05-12T04:07:27.322Z] + BUILD_STATUS=1

Comment 15

7 months ago
Backout by aciure@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0cb25b7bdee
Backed out 11 changesets (bug 1457560, bug 1366287) for causing Linux build bustages CLOSED TREE
(In reply to Jeff Walden [:Waldo] from comment #13)
> Hrm.  Certain builds on treeherder are complaining about
> 
>   TEST-UNEXPECTED-FAIL | check_stdcxx | We do not want these libstdc++
> symbol versions to be used:
> 
> referring to some of the new chrono symbols used here, and so I backed out
> the patch.  Not sure what's up, precisely.  However,
> https://searchfox.org/mozilla-central/source/toolkit/components/telemetry/
> Telemetry.cpp#1693 does not look too promising as to the usability of these
> symbols.  :-(  froyndj, you know what's up here, probably?

We check to see that the symbols we use from libstdc++ are sufficiently old such that our binaries will run with older systems; presumably some of the std::chrono code (being newer code) uses newer exports from libstdc++.so, and our test here is complaining about such symbols.

I think your only option here is to use the same workaround that telemetry uses.  TimeStamp would work for a monotonic time source, but I think you actually want some sort of numeric value here, which TimeStamp is unwilling to hand out.  (Another option is campaigning to bump our system requirements, but I don't think you want to get into that particular discussion just for this bug...)
Flags: needinfo?(nfroyd)
(In reply to Rick Waldron [:rwaldron] from comment #11)
> Do you think that might be sometime in the next week? If so, it can wait for
> that. 

Welp, guess we have to wait for this week even tho I could land last week.  :-(  I guess do the comment 17 thing and we can actually do this then.
Flags: needinfo?(jwalden+bmo) → needinfo?(waldron.rick)
(Assignee)

Comment 19

7 months ago
(In reply to Jeff Walden [:Waldo] from comment #18)
> (In reply to Rick Waldron [:rwaldron] from comment #11)
> > Do you think that might be sometime in the next week? If so, it can wait for
> > that. 
> 
> Welp, guess we have to wait for this week even tho I could land last week. 
> :-(  I guess do the comment 17 thing and we can actually do this then.


I can take a crack at revising this to use the same approach found in Telemetry. I'll ping for review!
Flags: needinfo?(waldron.rick)
(Assignee)

Comment 20

7 months ago
Jeff,

WDYT?


static bool
MonotonicNow(JSContext* cx, unsigned argc, Value* vp)
{
    CallArgs args = CallArgsFromVp(argc, vp);
    double now;

// As seen in toolkit/components/telemetry/Telemetry.cpp#1693
// This approach makes it safe to use the libstdc++ chrono
// symbols when compiling for newer targets.
#if defined(XP_UNIX) && !defined(XP_DARWIN)
    timespec ts;
    clock_gettime(CLOCK_REALTIME, &ts);
    now = ts.tv_sec * 1000 + ts.tv_nsec / 1000000;
#else
    using std::chrono::duration_cast;
    using std::chrono::milliseconds;
    using std::chrono::steady_clock;
    now = duration_cast<milliseconds>(steady_clock::now().time_since_epoch()).count();
#endif // XP_UNIX && !XP_DARWIN

    args.rval().setNumber(now);
    return true;
}



I compiled this both ways and the fallback is adequate.
(Assignee)

Comment 21

7 months ago
(In reply to Rick Waldron [:rwaldron] from comment #20)
> // As seen in toolkit/components/telemetry/Telemetry.cpp#1693

I've removed the line number from this ;)
(In reply to Rick Waldron [:rwaldron] from comment #20)
> #if defined(XP_UNIX) && !defined(XP_DARWIN)
>     timespec ts;
>     clock_gettime(CLOCK_REALTIME, &ts);
>     now = ts.tv_sec * 1000 + ts.tv_nsec / 1000000;

Shouldn't this try to use |CLOCK_MONOTONIC| instead of |CLOCK_REALTIME| if possible? For example like:
---
// Use a monotonic clock if available.
timespec ts;
if (clock_gettime(CLOCK_MONOTONIC, &ts) != 0) {
    if (clock_gettime(CLOCK_REALTIME, &ts) != 0) {
        JS_ReportErrorASCII(cx, "Cannot retrieve system clock");
        return false;
    }
}
---
(Assignee)

Comment 23

7 months ago
(In reply to André Bargull [:anba] from comment #22)
> (In reply to Rick Waldron [:rwaldron] from comment #20)
> > #if defined(XP_UNIX) && !defined(XP_DARWIN)
> >     timespec ts;
> >     clock_gettime(CLOCK_REALTIME, &ts);
> >     now = ts.tv_sec * 1000 + ts.tv_nsec / 1000000;
> 
> Shouldn't this try to use |CLOCK_MONOTONIC| instead of |CLOCK_REALTIME| if
> possible? For example like:
> ---
> // Use a monotonic clock if available.
> timespec ts;
> if (clock_gettime(CLOCK_MONOTONIC, &ts) != 0) {
>     if (clock_gettime(CLOCK_REALTIME, &ts) != 0) {
>         JS_ReportErrorASCII(cx, "Cannot retrieve system clock");
>         return false;
>     }
> }
> ---


Sure! I suppose I was taking the suggestion to use the approach in Telemetry a bit too literally. 


WDYT?



static bool
MonotonicNow(JSContext* cx, unsigned argc, Value* vp)
{
    CallArgs args = CallArgsFromVp(argc, vp);
    double now;

// As seen in toolkit/components/telemetry/Telemetry.cpp
// This approach makes it safe to use the libstdc++ chrono
// symbols when compiling for newer targets.
#if defined(XP_UNIX) && !defined(XP_DARWIN)
    timespec ts;
    // Use a monotonic clock if available.
    if (clock_gettime(CLOCK_MONOTONIC, &ts) != 0) {
        // Use a realtime clock as a fallback
        if (clock_gettime(CLOCK_REALTIME, &ts) != 0) {
            // Fail if no clock is available
            JS_ReportErrorASCII(cx, "Cannot retrieve system clock");
            return false;
        }
    }
    now = ts.tv_sec * 1000 + ts.tv_nsec / 1000000;
#else
    using std::chrono::duration_cast;
    using std::chrono::milliseconds;
    using std::chrono::steady_clock;
    now = duration_cast<milliseconds>(steady_clock::now().time_since_epoch()).count();
#endif // XP_UNIX && !XP_DARWIN

    args.rval().setNumber(now);
    return true;
}
Whoops, missed the comment earlier.  (Tacking on a needinfo? is the best way to set a flag that I'll definitely notice.)

I'd replace the comment above the #if with something more like

"""
The std::chrono symbols are too new to be present in STL on all the platforms we care about, so use raw POSIX clock APIs when it might be necessary.
"""

but otherwise, assuming that's basically copypasta from that location, looks fine.  Feel free to post that as a new patch.
(Assignee)

Comment 25

7 months ago
Thanks Jeff!
(Assignee)

Comment 26

7 months ago
Created attachment 8981978 [details] [diff] [review]
0001-Bug-1457560-Expose-262.agent.monotonicNow-for-Test26.patch
Attachment #8974769 - Attachment is obsolete: true
Attachment #8981978 - Flags: review?(jwalden+bmo)
Comment on attachment 8981978 [details] [diff] [review]
0001-Bug-1457560-Expose-262.agent.monotonicNow-for-Test26.patch

Review of attachment 8981978 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/builtin/TestingFunctions.cpp
@@ +5031,5 @@
> +    timespec ts;
> +    // Use a monotonic clock if available.
> +    if (clock_gettime(CLOCK_MONOTONIC, &ts) != 0) {
> +        // Use a realtime clock as a fallback
> +        if (clock_gettime(CLOCK_REALTIME, &ts) != 0) {

I'm hesitant about this fallback.  If it really ever is invoked, which I suppose probably it is just because standard libraries are awful, it seems like a serious issue if it returns a non-monotonic value.  I think we should work around this with manually enforced monotonicity in that case.  So something like

#if defined...
    auto ComputeNow = [](const timespec& ts) {
        return ts.tv_sec * 1000 + ts.tv_nsec / 1000000;
    };

    timespec ts;
    if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {
        // Use a monotonic clock if available.
        now = ComputeNow(ts);
    } else {
        // Use a realtime clock as fallback.
        if (clock_gettime(CLOCK_REALTIME, &ts) != 0) {
            JS_ReportErrorASCII(cx, "cannot retrieve system clock");
            return false;
        }

        now = ComputeNow(ts);

        // Manually enforce atomicity on a non-monotonic clock.
        {
            static mozilla::Atomic<bool, mozilla::ReleaseAcquire> spinLock;
            while (!spinLock.compareExchange(false, true))
                continue;

            static double lastNow = FLT_MIN;
            now = lastNow = std::max(now, lastNow)

            spinLock = false;
        }
    }
#else
    ...std approach...
#endif
Attachment #8981978 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 28

6 months ago
Sure, I can do that.
(Assignee)

Comment 29

6 months ago
Created attachment 8983785 [details] [diff] [review]
0001-Bug-1457560-Expose-262.agent.monotonicNow-for-Test26.patch

Updated patch, per latest review
Attachment #8981978 - Attachment is obsolete: true
Flags: needinfo?(jwalden+bmo)
Attachment #8983785 - Flags: review?(jwalden+bmo)

Comment 30

6 months ago
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/51238c5a88c5
Expose $262.agent.monotonicNow() for Test262 use in testing Atomic operation timeouts.  r=jwalden
Comment on attachment 8983785 [details] [diff] [review]
0001-Bug-1457560-Expose-262.agent.monotonicNow-for-Test26.patch

Review of attachment 8983785 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, modulo a couple things try pointed out (like my missing a ';' at the end of one line, using FLT_MIN instead of -FLT_MAX [this one I saw by manually debugging through the fallback code], making sure to #include the right header for FLT_MAX, etc.).  Landed -- thanks!

Oh, also -- the way test262-host.js works, it more or less tries to do all its stuff in a way that's resilient against the test mucking with it.  So monotonicNow is cached off the global in the function, rather than being captured in the global scope where it could be overwritten.  Probably not an issues for test262 tests, but the more this stuff doesn't break when tests try to break things, the less pain for someone at some distant future time.
Attachment #8983785 - Flags: review?(jwalden+bmo) → review+

Comment 32

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/51238c5a88c5
Status: NEW → RESOLVED
Last Resolved: 6 months ago
status-firefox62: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62

Updated

6 months ago
Flags: needinfo?(jwalden+bmo)

Comment 33

6 months ago
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/44508edc10b4
Remove a |false &&| inadvertently left in a patch as part of testing a fallback code path.  Followup to bug 1457560, r=me, thanks to IRC for pointing it out recently
You need to log in before you can comment on or make changes to this bug.