Implement CalculateProcessCreationTimestamp() on DragonFly/FreeBSD/NetBSD

RESOLVED FIXED in mozilla18

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jbeich, Unassigned)

Tracking

Trunk
mozilla18
x86_64
FreeBSD
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

6 years ago
Created attachment 659485 [details]
sample program to test ifdef maze
(Reporter)

Comment 1

6 years ago
Created attachment 659486 [details] [diff] [review]
Bug 789693 - Unify CalculateProcessCreationTimestamp() on OS X and BSDs.

The approach tries to address comment 2 in bug 633193 by sharing OS X code. It may be better than having 5 slightly different implementations racing for bitrot.

Tested as a sample program by me on FreeBSD and DragonFly, by :gaston on OpenBSD and NetBSD. Only Mac OS X wasn't tested.
Attachment #659486 - Flags: review?(taras.mozilla)

Comment 2

6 years ago
Comment on attachment 659486 [details] [diff] [review]
Bug 789693 - Unify CalculateProcessCreationTimestamp() on OS X and BSDs.

I would like to structure this define/ifdef mess differently. 
For example
+#if defined(__NetBSD__)
+  int mib[6] = { CTL_KERN, KERN_PROC2, KERN_PROC_PID, getpid(), sizeof(struct kinfo_proc2), 1 }
+#elif defined(__OpenBSD__)
+  int mib[6] = { CTL_KERN, KERN_PROC, KERN_PROC_PID, getpid(), sizeof(struct kinfo_proc), 1 };
+#else
   int mib[4] = { CTL_KERN, KERN_PROC, KERN_PROC_PID, getpid() };
+#endif


should just setup a dictinary of different field names and do 
#if OPENBSD
#define KINFO_PROC kinfo_proc
#elseif NETBSD
#define KINFO_PROC kinfo_proc2

 int mib[6] = { CTL_KERN, KERN_PROC, KERN_PROC_PID, getpid(), sizeof(KINFO_PROC), 1 };


-  struct kinfo_proc *proc = (kinfo_proc*) malloc(buffer_size);  
+#if defined(__NetBSD__)
+  struct kinfo_proc2 *proc = (struct kinfo_proc2 *) malloc(buffer_size);
+#else
+  struct kinfo_proc *proc = (struct kinfo_proc *) malloc(buffer_size);
+#endif
this mess would also be fixed by above

+
+#if defined(__NetBSD__) || defined(__OpenBSD__)
+  if (sysctl(mib, 6, proc, &buffer_size, NULL, 0)) {
+#else
   if (sysctl(mib, 4, proc, &buffer_size, NULL, 0)) {
+#endif
instead of using the 6 or 4 constant use
sizeof(mib)/sizeof(mib[0])

+#elif defined(__DragonFly__)
+  PRTime starttime = static_cast<PRTime>(proc->kp_start.tv_sec) * PR_USEC_PER_SEC;
+  starttime += proc->kp_start.tv_usec;
+#elif defined(__FreeBSD__)
+  PRTime starttime = static_cast<PRTime>(proc->ki_start.tv_sec) * PR_USEC_PER_SEC;
+  starttime += proc->ki_start.tv_usec;
+#else
   PRTime starttime = static_cast<PRTime>(proc->p_ustart_sec) * PR_USEC_PER_SEC;
   starttime += proc->p_ustart_usec;
+#endif

Similarly, above just define away the different structures like proc->kp_start.tv_sec instead of copying logic.
Attachment #659486 - Flags: review?(taras.mozilla) → review-
(Reporter)

Comment 3

6 years ago
Created attachment 659569 [details] [diff] [review]
Bug 789693 - Unify CalculateProcessCreationTimestamp() on OS X and BSDs.

(In reply to Taras Glek (:taras) from comment #2)
> I would like to structure this define/ifdef mess differently.

v2. no malloc/free, less ifdefs + block of macros (bigger than the code itself).

 #undef KERN_PROC still looks ugly but the alternative is

     int mib[] = {
     ...
   #if defined(__NetBSD__)
       KERN_PROC2,
   #else
       KERN_PROC,
   #endif
     ...
     };
Attachment #659486 - Attachment is obsolete: true
Attachment #659569 - Flags: review?(taras.mozilla)
(Reporter)

Comment 4

6 years ago
Created attachment 659570 [details]
sample program to test ifdef maze, v2
Attachment #659485 - Attachment is obsolete: true

Comment 5

6 years ago
Comment on attachment 659569 [details] [diff] [review]
Bug 789693 - Unify CalculateProcessCreationTimestamp() on OS X and BSDs.

Thanks for the quick revision.

Sheriffs, please push this to try before checkin
Attachment #659569 - Flags: review?(taras.mozilla) → review+

Updated

6 years ago
Keywords: checkin-needed

Comment 6

6 years ago
Comment on attachment 659569 [details] [diff] [review]
Bug 789693 - Unify CalculateProcessCreationTimestamp() on OS X and BSDs.

sorry, didn't notice that this was doing #define magic above #include calls. Please move #include calls up above defines.
Attachment #659569 - Flags: review+ → review-
(Reporter)

Comment 7

6 years ago
Created attachment 659675 [details] [diff] [review]
Bug 789693 - Unify CalculateProcessCreationTimestamp() on OS X and BSDs.

Like this:

   #if defined(__DragonFly__) || defined(__FreeBSD__)
   #include <sys/user.h>
   #endif

  +#include "mozilla/Telemetry.h"
  +#include "mozilla/StartupTimeline.h"
  +
   #if defined(__NetBSD__)
   #undef KERN_PROC
  [...]
   #define KP_START_USEC p_ustart_usec
   #endif

  -#include "mozilla/Telemetry.h"
  -#include "mozilla/StartupTimeline.h"
  -
   static NS_DEFINE_CID(kAppShellCID, NS_APPSHELL_CID);

   #define kPrefLastSuccess "toolkit.startup.last_success"
   #define kPrefMaxResumedCrashes "toolkit.startup.max_resumed_crashes"
   #define kPrefRecentCrashes "toolkit.startup.recent_crashes"

   #if defined(XP_WIN)
   #include "mozilla/perfprobe.h"
   
or further below, e.g. after XP_WIN ifdef ?
Attachment #659569 - Attachment is obsolete: true
Attachment #659675 - Flags: review?(taras.mozilla)
Keywords: checkin-needed

Updated

6 years ago
Attachment #659675 - Flags: review?(taras.mozilla) → review+
(Reporter)

Comment 8

6 years ago
Needs a Try push first per comment 5.
Keywords: checkin-needed
Looks green.

https://hg.mozilla.org/integration/mozilla-inbound/rev/61486f816524
Flags: in-testsuite-
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/61486f816524
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.