1419333 - Consider removing posix NSPR emulation

Status: NEW

(Core :: JavaScript Engine, enhancement, P3)

Description

[Jan de Mooij [:jandem]]

This was added in bug 931151 because building NSPR used to be very painful back then. However, nowadays we have --enable-nspr-build and that seems to work well.

What would break if we made --enable-nspr-build the default?

Comment 1

[Jan de Mooij [:jandem]]

What do you think?

This came up because bug 1410528 broke PosixNSPR shell builds on OS X (and presumably other platforms).

Comment 2

[Steve Fink [:sfink] [:s:]]

I would love it if we could remove NSPR entirely. With all the threading stuff moved out, it seems like we're getting pretty close.

Removing nspr would simplify embedding and would remove some messy configure logic and confusing command line options. And it would remove the problem of using a system nspr that's too old; I've run into crashes from that.

Unfortunately, nobody's working on this anymore. terrence was championing it while he was here.

The last major sticking point I'm aware of is PR_LoadLibrary and friends. I looked at the #ifdef mess that NSPR uses for those, and it was frightening. Now that we've unsupported so many older OSes, though, it might be a lot easier.

I don't really have any problem with removing the posix nspr stuff. Though it might be nice to have a single file that relies on NSPR -- kind of invert PosixNSPR, define js::* functions that call NSPR for you, so that anyone who wants to could incrementally remove NSPR usage from that file and it would be clear when they're all gone.

Comment 3

[Jan de Mooij [:jandem]]

So looking at PosixNSPR.h/cpp our NSPR dependencies right now are:

* PR_GetOpenFileInfo
* PR_CreateFileMap
* PR_MemMap, PR_MemUnmap
* PR_CloseFileMap

And some unused functions we could remove: PR_FileDesc2NativeHandle, PR_Seek.

I skimmed the NSPR code of each of these and it doesn't seem *too* hard to implement each of these on posix and windows. It's possible I missed something; NSPR code is really hard to read.

Unfortunately we have APIs that take a PRFileDesc* argument so that's pretty annoying :/

There's also CTypes, but since we don't build that by default in the shell we could just have --enable-ctypes imply --enable-nspr-build.