Closed Bug 1350966 Opened 3 years ago Closed 2 years ago

Enable adaptive mutex on FreeBSD for NSPR

Categories

(NSPR :: NSPR, defect)

4.15
Unspecified
FreeBSD
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jbeich, Assigned: jbeich)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

PTHREAD_MUTEX_ADAPTIVE_NP is supported since FreeBSD 7.0. Anything older than 10.3 is no longer supported downstream. No difference running tests.

https://github.com/freebsd/freebsd/commit/bbfd76f8722c569a2be8de7fd01318fa54fa750c
Attached patch v1Splinter Review
Attachment #8851643 - Flags: review?(kaie)
Comment on attachment 8851643 [details] [diff] [review]
v1

NSPR lacks a dedicated Try, so see on mozilla-central:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c1073a7e80ff
Comment on attachment 8851643 [details] [diff] [review]
v1

Let's try another reviewer.
Attachment #8851643 - Flags: review?(kaie) → review?(ted)
Blocks: 512076
Comment on attachment 8851643 [details] [diff] [review]
v1

I have resigned as NSPR module owner. Sorry for the inconvenience.
Attachment #8851643 - Flags: review?(ted)
Your try build didn't run any tests, are you sure that build-only is sufficient?

I'm fine with adding this FreeBSD specific define.

r=kaie after you either ran a try build with tests, or provided explanation why the build-only is sufficient.
(In reply to Kai Engert (:kaie:) from comment #5)
> Your try build didn't run any tests, are you sure that build-only is
> sufficient?

PTHREAD_MUTEX_ADAPTIVE_NP affects only performance on Linux and FreeBSD. If the build isn't broken one'd have a hard time testing it has any effect. However, in this case one can simply check ifdef logic standalone.

  $ cat a.c
  #if (defined(LINUX) && (__GLIBC__ > 2) || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 2)) || \
      (defined(FREEBSD) && __FreeBSD_version > 700055)
  #warning "using PTHREAD_MUTEX_ADAPTIVE_NP"
  #endif
  int main(int argc, char *argv[]) { return 0; }

  $ cc a.c

  # On FreeBSD 12.0
  $ cc -DFREEBSD -include osreldate.h a.c
  a.c:3:2: warning: "using PTHREAD_MUTEX_ADAPTIVE_NP" [-W#warnings]
  #warning "using PTHREAD_MUTEX_ADAPTIVE_NP"
   ^
  1 warning generated.

  # Mimic CentOS 6
  $ cc -DLINUX -D__GLIBC__=2 -D__GLIBC_MINOR__=12 a.c
  a.c:3:2: warning: "using PTHREAD_MUTEX_ADAPTIVE_NP" [-W#warnings]
  #warning "using PTHREAD_MUTEX_ADAPTIVE_NP"
   ^
  1 warning generated.

runtests.sh output shows no difference with/without the patch on FreeBSD.

unit tests (all): https://treeherder.mozilla.org/#/jobs?repo=try&revision=b1ee5761fb8d
talos (linux-only): https://treeherder.mozilla.org/#/jobs?repo=try&revision=6fbea3a8edda
Attachment #8851643 - Flags: review?(kaie)
Attachment #8851643 - Flags: review?(kaie) → review+
https://hg.mozilla.org/projects/nspr/rev/6b85368f877591e3c4defd9f201dedf0fdf1d928
Assignee: nobody → jbeich
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.16
You need to log in before you can comment on or make changes to this bug.