751727 - Remove nspr TimeStamp implementation

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

Attachment: 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.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2c694e6169d5

Comment 3

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/2c694e6169d5

Comment 4

[Landry Breuil (:gaston)]

It seems this broke the build on OpenBSD where HAVE_CLOCK_MONOTONIC is not defined.

Comment 5

[Landry Breuil (:gaston)]

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.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

(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.

Comment 7

[Landry Breuil (:gaston)]

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..

Comment 8

[Landry Breuil (:gaston)]

OpenBSD got fixed in http://marc.info/?l=openbsd-cvs&m=133643021430806&w=2, and the HAVE_CLOCK_MONOTONIC test passes fine now.

Comment 9

[Landry Breuil (:gaston)]

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.

Comment 10

[Jeff Muizelaar [:jrmuizel]]

(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.

Comment 11

[Landry Breuil (:gaston)]

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 ?

Comment 12

[Jeff Muizelaar [:jrmuizel]]

(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.

Comment 13

[Landry Breuil (:gaston)]

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..