Hook spidermonkey into rust's allocator

[michaelwu]

This is the alternative to #61 . Rust doesn't quite provide all the APIs necessary to do this fully - there is no calloc equivalent, but it's probably fine in most cases.

fake_rustalloc is provided to link the js shell and tests, but isn't used anywhere else.

@asajeffrey - do you mind seeing if this works as well as turning on jemalloc?

r? @Manishearth

[Manishearth]

Where do we link this to Rust's jemalloc?

[michaelwu]

We don't - this is only used to make standalone binaries link successfully in SM. Otherwise, tests and js shell don't link.

[Manishearth]

So when will things link to Rust's jemalloc?

[michaelwu]

To elaborate a bit more - fake_rustalloc is used for linking C++ binaries that are only used for testing. These binaries are built by SM using a C++ compiler. In these cases, fake_rustalloc just hooks these binaries up to the system allocator. In normal cases however, fake_rustalloc isn't used and rustc properly links SM against rust's allocator.

[Manishearth]

Ah, so the cpp and header file below are only used for testing? In case we're using rustc, it will automatically link against jemalloc's malloc/calloc/realloc symbols? Sounds good.

So what's the situation right now? If the linking happens in the final link step, why is SM not using jemalloc (which is being linked against) now?

[michaelwu]

SM is currently using js_malloc/js_realloc/js_free, which redirects to the system malloc/realloc/free. Rust's copy of jemalloc uses je_malloc/jerealloc/je_free naming, so it doesn't automatically override other allocators.

[Manishearth]

(figured it out after chatting in IRC; only fake_rustalloc is for testing, jscustomallocator is not)

[Manishearth]

r=me once @asajeffrey gets a chance to play with it

[asajeffrey]

I'll have a look at this tomorrow.

[asajeffrey]

Oh okay, maybe the day after tomorrow.

[asajeffrey]

Got a segfault (posted to irc, crossposting here for posterity):

#0  0x0000555557123576 in je_sdallocx ()
#1  0x0000555556c44c11 in js_free (p=0x0) at /home/ajeffrey/github/servo/mozjs/rust_mem/jscustomallocator.h:62
#2  js::ScopedFreePtrTraits<unsigned char>::release (ptr=0x0) at ../../dist/include/js/Utility.h:303
#3  mozilla::Scoped<js::ScopedFreePtrTraits<unsigned char> >::~Scoped (this=<synthetic pointer>, __in_chrg=<optimized out>) at ../../dist/include/mozilla/Scoped.h:107
#4  js::ScopedJSFreePtr<unsigned char>::~ScopedJSFreePtr (this=<synthetic pointer>, __in_chrg=<optimized out>) at ../../dist/include/js/Utility.h:305
#5  js::NewStringCopyNDontDeflate<(js::AllowGC)0, unsigned char> (cx=cx@entry=0x7fffccc87000, s=s@entry=0x555557376370 <js_CollatorCompareGet_str> "Intl_Collator_compare_get", 
    n=n@entry=25) at /home/ajeffrey/github/servo/mozjs/mozjs/js/src/vm/String.cpp:1116
#6  0x0000555556c44d15 in js::NewStringCopyN<(js::AllowGC)0, unsigned char> (cx=cx@entry=0x7fffccc87000, 
    s=s@entry=0x555557376370 <js_CollatorCompareGet_str> "Intl_Collator_compare_get", n=n@entry=25) at /home/ajeffrey/github/servo/mozjs/mozjs/js/src/vm/String.cpp:1150
#7  0x0000555556a8d570 in AtomizeAndCopyChars<unsigned char> (ib=js::InternAtom, length=25, tbchars=0x555557376370 <js_CollatorCompareGet_str> "Intl_Collator_compare_get", 
    cx=0x7fffccc87000) at /home/ajeffrey/github/servo/mozjs/mozjs/js/src/jsatom.cpp:350
#8  js::Atomize (cx=cx@entry=0x7fffccc87000, bytes=0x555557376370 <js_CollatorCompareGet_str> "Intl_Collator_compare_get", length=25, ib=ib@entry=js::InternAtom)
    at /home/ajeffrey/github/servo/mozjs/mozjs/js/src/jsatom.cpp:425
#9  0x0000555556a8e284 in JSRuntime::initializeAtoms (this=this@entry=0x7fffccc40000, cx=cx@entry=0x7fffccc87000) at /home/ajeffrey/github/servo/mozjs/mozjs/js/src/jsatom.cpp:147
#10 0x0000555556e6e433 in js::NewContext (rt=0x7fffccc40000, stackChunkSize=<optimized out>) at /home/ajeffrey/github/servo/mozjs/mozjs/js/src/jscntxt.cpp:125
#11 0x000055555670da30 in rust::Runtime::new::h98f4f7f5d95588c8dCi ()
#12 0x00005555560df03b in script_thread::ScriptThread::new_rt_and_cx::h5ff56114013021957sj ()
#13 0x00005555562b2bf9 in script_thread::ScriptThread::new::h0d84950678ab1af1vqj ()
#14 0x00005555562b0111 in sys_common::unwind::try::try_fn::h15090099631519579870 ()
#15 0x0000555557111a5c in __rust_try ()
#16 0x000055555710e06e in sys_common::unwind::inner_try::hf5df2c554f3d837dFdt ()
#17 0x00005555562b1b7b in boxed::F.FnBox$LT$A$GT$::call_box::h6716255501184128001 ()
#18 0x0000555557113e84 in sys::thread::Thread::new::thread_start::h05e6a3d5b147b01bsCx ()
#19 0x00007ffff5c206aa in start_thread (arg=0x7fffcdbff700) at pthread_create.c:333
#20 0x00007ffff573eeed in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109

[michaelwu]

It turns out the allocator API that rust exposes and the one SM wants are more incompatible than expected. Going to see if I can make the SM allocator API fully sized. If so, we can come back to hooking SM into Rust's allocator after the next smup.

[michaelwu]

rust-lang/rust#31976 adds support for unsized deallocation.

[perlun]

@michaelwu - has this gone stale? Will it be finished?

[metajack]

@perlun I believe that @michaelwu will not be continuing work on this. I would love it if someone saw this through to completion though.

[perlun]

@metajack - alright. I don't think I feel confident about doing it (yet), but maybe it would be worth for you to enumerate what you believe to be the remaining steps? So someone could more easily bring it forward.

[metajack]

@perlun I think the first step is to check if the Rust landscape changed since this was opened.

Rust doesn't quite provide all the APIs necessary to do this fully - there is no calloc equivalent, but it's probably fine in most cases.

and later

t turns out the allocator API that rust exposes and the one SM wants are more incompatible than expected. Going to see if I can make the SM allocator API fully sized.

Is that still true or likely to change soon? Maybe the first thing to do to answer those question is to write down what this API is on the SM side so we can starting discussing how to get there.

cc @asajeffrey @nikomatsakis @tschneidereit

[tschneidereit]

@fitzgen, this is something we should also have on the radar. We need either this or #61 for SpiderMonkey to be at its best.

[fitzgen]

From #61:

This speeds up SM performance significantly (about 2x on some Dromaeo JS tests).

I find this really surprising since the GC allocates in 2MiB chunks and then uses its own allocator, but I suppose there is still a lot of vm code that is using normal malloc.

---

Anyways, I haven't read through this backlog, but here is the status WRT allocators for my current smup work. I ported from the custom makefile majiggery to using js/src/devtools/autospider.py to drive the builds. Using the same infrastructure that firefox / sm / m-c's CI is using is a good thing on principle and means we'll have less breakage since there are more eyes on it. I'm not passing any configuration flags or options to autospider.py, so that means we will have the equivalent of #61: a dupicate copy of jemalloc.

I can look into sharing the same jemalloc some more once I complete the smup.

[fitzgen]

I find this really surprising since the GC allocates in 2MiB chunks and then uses its own allocator, but I suppose there is still a lot of vm code that is using normal malloc.

And array elements are backed by direct mallocs as well, and that seems to be the case that the benchmark is hitting.

[nox]

Many things changed since 2016, on both SpiderMonkey and Rust sides, closing this because I don't think it can be salvaged.