Closed Bug 751727 Opened 9 years ago Closed 9 years ago

Remove nspr TimeStamp implementation

Categories

(Core :: XPCOM, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: jrmuizel, Assigned: jrmuizel)

References

Details

Attachments

(1 file)

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+
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
Closed: 9 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.
(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.
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.
(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 ?
(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.