Closed
Bug 1457560
Opened 6 years ago
Closed 6 years ago
Expose $262.agent.monotonicNow() for Test262 use in testing Atomic operation timeouts
Categories
(Core :: JavaScript Engine, enhancement, P3)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: rwaldron, Assigned: rwaldron)
Details
Attachments
(1 file, 3 obsolete files)
5.31 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
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•6 years ago
|
Assignee: nobody → waldron.rick
Comment 1•6 years ago
|
||
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.)
Comment 2•6 years ago
|
||
(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•6 years 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!
Updated•6 years ago
|
Priority: -- → P3
Assignee | ||
Comment 4•6 years ago
|
||
Attachment #8972608 -
Flags: review?(jwalden+bmo)
Attachment #8972608 -
Flags: review?(jorendorff)
Comment 5•6 years ago
|
||
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 6•6 years ago
|
||
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•6 years 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•6 years ago
|
||
Updates per review
Attachment #8972608 -
Attachment is obsolete: true
Attachment #8974769 -
Flags: review?(jwalden+bmo)
Comment 9•6 years ago
|
||
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+
Comment 10•6 years ago
|
||
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•6 years 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•6 years 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
Comment 13•6 years ago
|
||
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)
Comment 14•6 years ago
|
||
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•6 years 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
Comment 16•6 years ago
|
||
Backed out for causing Linux build bustages. Pushes with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&fromchange=971159738904104edcbf6ffb41dca1c281fc28ee&tochange=f0cb25b7bdeefa070aac39697f0ecff54fa77699&selectedJob=178148203 Failure logs: https://treeherder.mozilla.org/logviewer.html#?job_id=178148203&repo=mozilla-inbound&lineNumber=32772 https://treeherder.mozilla.org/logviewer.html#?job_id=178148220&repo=mozilla-inbound&lineNumber=1734 https://treeherder.mozilla.org/logviewer.html#?job_id=178148220&repo=mozilla-inbound&lineNumber=1734 https://treeherder.mozilla.org/logviewer.html#?job_id=178149463&repo=mozilla-inbound&lineNumber=6904 https://treeherder.mozilla.org/logviewer.html#?job_id=178149764&repo=mozilla-inbound&lineNumber=1797 Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/f0cb25b7bdee
Flags: needinfo?(jwalden+bmo)
Comment 17•6 years ago
|
||
(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)
Comment 18•6 years ago
|
||
(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•6 years 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•6 years 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•6 years 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 ;)
Comment 22•6 years ago
|
||
(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•6 years 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; }
Comment 24•6 years ago
|
||
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•6 years ago
|
||
Thanks Jeff!
Assignee | ||
Comment 26•6 years ago
|
||
Attachment #8974769 -
Attachment is obsolete: true
Attachment #8981978 -
Flags: review?(jwalden+bmo)
Comment 27•6 years ago
|
||
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 years ago
|
||
Sure, I can do that.
Assignee | ||
Comment 29•6 years ago
|
||
Updated patch, per latest review
Attachment #8981978 -
Attachment is obsolete: true
Flags: needinfo?(jwalden+bmo)
Attachment #8983785 -
Flags: review?(jwalden+bmo)
Comment 30•6 years 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 31•6 years ago
|
||
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 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/51238c5a88c5
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Updated•6 years ago
|
Flags: needinfo?(jwalden+bmo)
Comment 33•6 years 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
Comment 34•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/44508edc10b4
You need to log in
before you can comment on or make changes to this bug.
Description
•