Closed Bug 577056 Opened 14 years ago Closed 5 months ago

fatval: Memory layout for 64-bit Solaris is different than other 64-bit systems.

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
x86
Solaris
Type:
defect

Tracking

()

Status:
RESOLVED WONTFIX

People

(Reporter: leon.sha, Unassigned)

References

Details

Memory layout for 64-bit Solaris is different than other 64-bit systems.
http://developers.sun.com/solaris/articles/solaris_memory.html
User space memory may locate on PART-A (0xFFFFFD80.00000000 - 0xFFFF8000.00000000) and PART-B (0x00008000.00000000 - 0x00000000.04000000).
As we tried, mmap() will return the address in PART-A, while malloc() will return the address in PART-B. So for 64-bit solaris, we may need a bit to indicata if the memory is in PART-A or PART-B.
Good observation!  Because the only pointers to heap memory that are stuffed into jsvals are GC-things (and statically allocated JSStrings, more on that in a second), and the allocation of GC-things funnels through platform-specific code in jsgcchunk.cpp, it seems like you can add a special Solaris path that only allocates in one of those two address ranges.  Although there may be a perf boost to mmap(), using malloc() makes Solaris no different than other 64-bit platforms and thus would be *massively* simpler, so I suggest trying that out first and measuring the perf.

As for the statically allocated JSStrings, Wes demonstrated that they seem to hang out in low addresses that fit in the 47-bit payload.  This is another reason to use malloc() for GC-things, since that way both GC-things and static strings 0 extend the high 17-bits of the payload.
Leon, thanks for pointing out that memory model. The hole in the middle gives us more options than I thought we'd have.

Luke points out that the tracing JIT and the method JIT directly emit intermediate-language code which decomposes the jsval, rather than calling into a common subroutine. This suggests that adding another flag to the jsval might be both tricky and an ongoing maintenance nuisance; that is where Luke's suggestion to investigate using malloc really starts to make sense.

It might also be worthwhile using libumem (Solaris' slab allocator) with the sbrk backend.  This will yield the same memory address ranges as malloc, but hopefully with better fragmentation characteristics.  Changing jsgcchunk.cpp to call the libumem functions (e.g. umem_alloc) rather than mmap on Solaris should "just work" and give good MT perf. Measuring the performance will be interesting; I have heard claims in the past that this is a really good allocator.

If Firefox on Solaris is built with --enable-jemalloc it would need similar patches... although libumem alone would probably be faster than jemalloc built on libumem.
Ideally, we should let user to choose memory allocator libraries through LD_PRELOAD.
Then malloc could be sbrk or mmap.

If we want to disallow LD_PRELOAD to override malloc(), we have to static link malloc functions into libmozjs.so.
I agree that being able to override malloc is a useful trick - especially in light of the debug tools available this way.

I question, however, whether the increased maintenance complexity -- having a separate implementation fat jsval decode/encode for 64-bit Solaris -- is worth having the flexibility of allocating memory with mmap. Especially given that the JITs emit this stuff directly rather than going through a central API. 

Allowing arbitrary mmap'd addresses (i.e. above the VM hole) also adds a couple of extra instructions for each jsval conversion, which I would expect to be measurable.

Note that restricting the allocator for jsgcchunk to something using sbrk (e.g. libumem functions rather than mmap) does not prevent other parts of the code which use malloc() from being overridden with LD_PRELOAD. The only thing that matters, I believe, is allocation address for JS GC Things.
(In reply to comment #4)
> Note that restricting the allocator for jsgcchunk to something using sbrk (e.g.
> libumem functions rather than mmap) does not prevent other parts of the code
> which use malloc() from being overridden with LD_PRELOAD. The only thing that
> matters, I believe, is allocation address for JS GC Things.

I'm not sure about this. According to the man page, it seems it is undefined behavior.

USAGE
     The behavior of brk() and sbrk() is unspecified if an appli-
     cation  also  uses  any  other  memory  functions  (such  as
     malloc(3C), mmap(2), free(3C)).
(In reply to comment #1)
> Good observation!  Because the only pointers to heap memory that are stuffed
> into jsvals are GC-things (and statically allocated JSStrings, more on that in
> a second)

This is wrong. JS API also allows to stuff arbitrary pointer as a private value. That we do not control at all.
(In reply to comment #6)
> (In reply to comment #1)
> > Good observation!  Because the only pointers to heap memory that are stuffed
> > into jsvals are GC-things (and statically allocated JSStrings, more on that in
> > a second)
> 
> This is wrong. JS API also allows to stuff arbitrary pointer as a private
> value. That we do not control at all.

Yes, but private pointers guarantee that the low bit is unset which is enough to shift right, making the private look like a double.
(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #1)
> > > Good observation!  Because the only pointers to heap memory that are stuffed
> > > into jsvals are GC-things (and statically allocated JSStrings, more on that in
> > > a second)
> > 
> > This is wrong. JS API also allows to stuff arbitrary pointer as a private
> > value. That we do not control at all.
> 
> Yes, but private pointers guarantee that the low bit is unset which is enough
> to shift right, making the private look like a double.

You are right - there is no problems with private values as long as we can re-shuffle them as a double.
For record, if I managed to get mmap chunk, heap and static JSStrings allocated at addresses between 0x00000000.00000000 and 0x00008000.00000000, we are still using user stack in jsobj.cpp.

e.g.
2251         Value dummy;
2252         if (!CallJSPropertyOp(cx, obj2->getClass()->delProperty, obj2, desc.id, &dummy))
2253             return false;
2254     }

2336     bool rval;
2337     if (!DefineProperty(cx, obj, *desc, true, &rval))
2338         return false;

2361     /* 15.2.3.6 step 4 */
2362     JSBool junk;
2363     return js_DefineOwnProperty(cx, obj, nameidr.id(), descval, &junk);
I've been talking about this in the channel w/ Wes. I'm not too confident that a solution will be found for this that will work for everyone. Just to raise my hand in the crowd here.. we use SM on Solaris(Sparc), AIX(Power), and HP-UX(IA64) in 32-bit right now. We'd like to be able to run in 64-bit in the future when this is ironed out, but we also have other restrictions.. In my world:

- SM itself never touches the system heap (malloc, mmap, sbrk, etc)
- SM is modified to only use our custom allocators
- Memory from our custom allocators *must* come from mmap

This doesn't leave a whole lot of cross-platform options. 128-bit jsval?
Note, a 128-bit jsval for 64-bit architectures is being considered even on x86_64 systems.  sstangl was experimenting with this.
Even though I know the existing jsval layout prevents 64-bit from working on Solaris & friends at runtime, I'd still like the 64-bit build to work so we can ensure the rest of the code still compiles. Unless anyone has objections, I want to land patches that use the existing jsval layout to add support so 64-bit big-endian Solaris builds out of the box. I got the shell building in this mode by:

- Defining the jsval layout (bug 618485)
- Manually passing -DAVMPLUS_64BIT at configure time (configure would need to be modified to enable this when building in 64-bit mode. No bug for this yet, but I could file one with a patch for this.)
- Implementing enough of tracejit support so that it compiles (I did this in my tree, adding asm_qbinop and asm_immq and stubbing out the other ones until I get to them). I'll get this cleaned up and either post it to the other bug or create a new one. Initial patch is here: http://pastebin.mozilla.org/891704
(In reply to comment #12)
> - Defining the jsval layout (bug 618485)
> - Manually passing -DAVMPLUS_64BIT at configure time (configure would need to
> be modified to enable this when building in 64-bit mode. No bug for this yet,
> but I could file one with a patch for this.)
> - Implementing enough of tracejit support so that it compiles (I did this in my
> tree, adding asm_qbinop and asm_immq and stubbing out the other ones until I
> get to them). I'll get this cleaned up and either post it to the other bug or
> create a new one. Initial patch is here: http://pastebin.mozilla.org/891704

You also need bug 589754 (which I think i was working on before the new jsval landed)
(In reply to comment #12)

> - Manually passing -DAVMPLUS_64BIT at configure time (configure would need to
> be modified to enable this when building in 64-bit mode. No bug for this yet,
> but I could file one with a patch for this.)

Here's what i use on OpenBSD/sparc64 for that (chunk of a larger patch for js/src/configure.in), but i doubt it applies to solaris in 64-bits mode.

@@ -2703,6 +2703,10 @@ arm*-*)
 sparc-*)
     AC_DEFINE(AVMPLUS_SPARC)
     ;;
+sparc64-*)
+    AC_DEFINE(AVMPLUS_SPARC)
+    AC_DEFINE(AVMPLUS_64BIT)
+    ;;
 esac

 case "$target" in
Assignee: general → nobody
Having a similar issue on Solaris sparc 64-bit (https://pastebin.mozilla.org/8920765), we worked around the issue by building the consumers of mozjs (gjs, and gnome-shell) seeded with SF1_SUNW_ADDR32 using a mapfile, thus restricting it to 32-bits.  In case it benefits others trying this on sparc 64-bit, this was the contents of our mapfile as suggested in https://docs.oracle.com/cd/E53394_01/html/E54813/man-icr.html

$mapfile_version 2
CAPABILITY {
        SF += ADDR32;
};
Severity: normal → S3
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.