[wk8]

With proper unit tests

[ignisf]

Odd, it's failing on Linux

#
# Fatal error in .././src/snapshot/serialize.h, line 725
# Check failed: IsSane().
#

[wk8]

@ignisf : dangit :( seems it failed on Travis too (for Linux as well).

How did you get that error message? I don't see that in Travis?

Trying on an Ubuntu VM.

[ignisf]

I'm no C++ programmer, but keep in mind that by default V8 on linux uses external snapshot data. We've worked around this issue by adding this flag to the V8 build process: https://github.com/cowboyd/libv8/blob/master/ext/libv8/builder.rb#L29-L30

That's about the only thing different I can think of between osx and linux. It could be something completely different.

[ignisf]

Relevant discussion: rubyjs/libv8#178 (comment)

[ignisf]

How did you get that error message? I don't see that in Travis?

ran bundle exec rake on my Arch Linux system.

[ignisf]

Hmm, I'm getting an /home/ignisf/Projects/mini_racer/lib/mini_racer.rb:93: [BUG] Illegal instruction at 0x007fbac6f91a09 though, and Travis build failure for 2.3.1 looks like a SEGFAULT...

[wk8]

@ignisf : thanks for all that, looking into it shortly :)

[ignisf]

Ok, I think I'm onto something here. I tried this https://developers.google.com/v8/get_started#run-the-example example on my box, getting this:

[ignisf@clarity mini_racer]$  g++ -I/home/ignisf/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/libv8-5.1.281.59.1-x86_64-linux/vendor/v8/include test.cxx -o hello_world -Wl,--start-group /home/ignisf/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/libv8-5.1.281.59.1-x86_64-linux/vendor/v8/out/x64.release/libv8_{base,libbase,external_snapshot,libplatform}.a -Wl,--end-group -lrt -ldl -pthread -std=c++0x
[ignisf@clarity mini_racer]$ ./hello_world 
Failed to open startup resource './natives_blob.bin'.
Failed to open startup resource './snapshot_blob.bin'.


#
# Fatal error in ../src/snapshot/natives-external.cc, line 122
# Check failed: holder_.
#

==== C stack trace ===============================

 1: 0x55d2e9f6e382
 2: 0x55d2e9f7399a
 3: 0x55d2e9aa552a
 4: 0x55d2e9aa5c34
 5: 0x55d2e9b21eeb
 6: 0x55d2e989c8ab
 7: 0x55d2e987a118
 8: __libc_start_main
 9: 0x55d2e9879f69
Illegal instruction (core dumped)
[ignisf@clarity mini_racer]$ 

[ignisf]

Copying those two files to the local directory allowed me to run the hello world. Here's a branch of libv8 for you to test with: https://github.com/cowboyd/libv8/commits/external-snapshot

[ignisf]

V8 requires its 'startup snapshot' to run. Copy the snapshot files to where your binary is stored:
cp out/x64.release/*.bin .

rofl

[wk8]

@ignisf : thanks a lot for your help, much appreciated. Testing your libv8 branch :)

[ignisf]

np, hopefully you can get to the bottom of this

[ignisf]

just one more thing before I call it a night -- help me grasp the impact of this issue. Am I correct to think that the so called snapshot's purpose is to speed up context creation. And significant performance improvement is expected to be observed in cases which require the creation of many contexts?

[wk8]

@ignisf : exactly. Our use case is: we have a base context that we could re-use across requests to do server-side pre-rendering. This would allow caching it once and for all

[wk8]

And I didn't come up with the name "snapshot", V8 did ;-) http://v8project.blogspot.com/2015/09/custom-startup-snapshots.html

[ignisf]

https://github.com/v8/v8/blob/5.1.281.59/include/v8.h#L6618-L6636

[wk8]

Thanks!

[wk8]

@ignisf : fixed, was just me being dumb and having done too little C++ recently :-/

Thanks for your help though!

[SamSaffron]

A couple of questions:

• Do we want to optionally allow to run the snapshot through WarmUpSnapshotDataBlob ? It should make it faster in theory
• What happens if there is a parse error in the snapshot, is there any way to debug?

Is
StartupData startup_data = V8::CreateSnapshotDataBlob(RSTRING_PTR(str));
+

• if (startup_data.data == NULL && startup_data.raw_size == 0) {

•    rb_raise(rb_eSnapshotError, "Could not create snapshot, most likely the source is incorrect");
  

• }

• snapshot_info->data = startup_data.data;
• snapshot_info->raw_size = startup_data.raw_size;

leaking a StartupData object somehow?

[wk8]

@SamSaffron : thanks for your review.

• WarmUpSnapshotDataBlob => good idea, happy to add that option, doing that today
• debug if error in snapshot => not easy to do, and CreateSnapshotDataBlob does not provide any feedback in that case, except if they do that à la errno and failed to document it (see http://v8.paulfryzel.com/docs/master/classv8_1_1_v8.html#a723f993d0fb7f408576c7143bb8dfa6e); I've found the best way to debug faulty snapshots is to try & eval them in an empty context, where V8 can then give more explicit error messages (happy to add that to the MiniRacer::SnapshotError exception's message if you want)
• leaking a StartupData object somehow => I don't think so, StartupData objects only contain raw data as a char* and its size as an int (see https://github.com/v8/v8/blob/1298aef51fc0e7569e1b9c5905ad8d29a2903246/include/v8.h#L6371-6375) so if the char* is NULL, as it is when the exception is thrown, then there's nothing to be leaked

[wk8]

@SamSaffron : snapshots warm up added (edeaa9d)

[ignisf]

Typo? Or am I missing the definition of the awesome sounding term "warping up"? :)

[wk8]

Er, thanks, fixing ;)

[LtSquigs]

Just a quick note on WarmUpSnapshotDataBlob, it appears to be mostly just to allow you to call functions to get them to compile (as noted in the comments of the function https://github.com/v8/v8/blob/master/src/api.cc#L520)

This is because functions are lazily compiled (electron/electron#169 (comment)), the solution in that thread from the creator of snapshots was just to call the functions but wrap them in try/catch to force them to compile. WarmUpSnapshotDataBlob just seems to be a more formal way to solve that particular issue.

[SamSaffron]

OK I will merge this in, @wk8 since we are getting all fancy here one area that is very weak atm is memory management and visibility, I would love to get some stuff in here.

#18

at a minimum visibility on what is currently in use by v8, but more predictable v8 disposal is also quite important (and especially for the snapshot usage, cause you want to dispose the v8 resources asap once you are done)

[wk8]

Thanks for merging!