Remove nspr TimeStamp implementation

RESOLVED FIXED in mozilla15

Status

()

Core
XPCOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jrmuizel, Assigned: jrmuizel)

Tracking

unspecified
mozilla15
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 620865 [details] [diff] [review]
Remove nspr TimeStamp implementation

PR_IntervalNow() is not monotonic and so we shouldn't be using it for implementing TimeStamp, especially now that it is exposed to web content (Bug 539095).

Furthermore, it seems that the nspr implementation is being used unintentionally on some platforms. For example, we used to use it on Android and just now I see that it is being used in FF12 on Ubuntu.
Attachment #620865 - Flags: review?(roc)
Comment on attachment 620865 [details] [diff] [review]
Remove nspr TimeStamp implementation

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

Fine, as long as this doesn't break any builds.
Attachment #620865 - Flags: review?(roc) → review+
(Assignee)

Comment 2

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c694e6169d5
Assignee: nobody → jmuizelaar
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/2c694e6169d5
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
It seems this broke the build on OpenBSD where HAVE_CLOCK_MONOTONIC is not defined.
After a quick look configure fails to find CLOCK_MONOTONIC #define because it's in sys/time.h and not time.h. I'll file a followup bug to fix configure.in.
(Assignee)

Comment 6

5 years ago
(In reply to Landry Breuil from comment #5)
> After a quick look configure fails to find CLOCK_MONOTONIC #define because
> it's in sys/time.h and not time.h. I'll file a followup bug to fix
> configure.in.

Great. This is exactly the kind of thing this patch was supposed to catch.
Depends on: 752280
Hrm apparently POSIS says that it should be in time.h, so i'll see if i can get OpenBSD fixed instead. Fwiw, i've sent a sys/time.h fix to try here : https://tbpl.mozilla.org/?tree=Try&rev=15aea6efea09
Apparently there wasnt related breakage..
OpenBSD got fixed in http://marc.info/?l=openbsd-cvs&m=133643021430806&w=2, and the HAVE_CLOCK_MONOTONIC test passes fine now.
Thinking more about it.. maybe the configure check for clock_gettime(CLOCK_MONOTONIC) should fail configure itself now, so that linking doesnt fail later with missing syms.
(Assignee)

Comment 10

5 years ago
(In reply to Landry Breuil (:gaston) from comment #9)
> Thinking more about it.. maybe the configure check for
> clock_gettime(CLOCK_MONOTONIC) should fail configure itself now, so that
> linking doesnt fail later with missing syms.

That would definitely be an improvement.
Ok, then two questions :
- would "[Your system doesn't support clock_gettime(CLOCK_MONOTONIC) or its detection failed]" be a warning clear enough ?
- should i go all the way and also remove all references to HAVE_CLOCK_MONOTONIC which is now mandatory true ?
(Assignee)

Comment 12

5 years ago
(In reply to Landry Breuil (:gaston) from comment #11)
> Ok, then two questions :
> - would "[Your system doesn't support clock_gettime(CLOCK_MONOTONIC) or its
> detection failed]" be a warning clear enough ?
> - should i go all the way and also remove all references to
> HAVE_CLOCK_MONOTONIC which is now mandatory true ?

I would prefer getting rid of HAVE_CLOCK_MONOTONIC all together.
I've tried that approach in https://hg.mozilla.org/try/rev/8f0371a839db  (see https://tbpl.mozilla.org/?tree=Try&rev=a3e9ca6b8c0e ) but that broke mac os x..

Something is still fishy because the test succeeds on linux/bsd but fails on macos, Previously xpcom/ds/Makefile.in was reyling on that since _posix.cpp was used if the test succeeded and then macos implem was used otherwise, so making it a failure doesnt seem to work. Interestingly, from try logs clock_gettime(CLOCK_MONOTONIC) test is not run at all on windows. Of course i dont have a mac to test the clock_gettime() configure test failure..
You need to log in before you can comment on or make changes to this bug.