rubyjs / therubyracer

Embed the V8 Javascript Interpreter into Ruby
1.66k stars 191 forks source link

Implement numbers #353

Closed cowboyd closed 9 years ago

cowboyd commented 9 years ago

Note: This was originally opened as https://github.com/stormbreakerbg/therubyracer/pull/4

This implements the JavaScript types Number, Int32, and Uint32. Previously, I had been cheating and always using low-level ruby types everywhere and then converting them to their v8 equivalents at the last minute in the C code. Now that this properly wraps numeric types, If a method returns an Int32, then ruby will see it as a V8::C::Int32 and not a Fixnum etc..

Not only does this represent the v8 API as faithfully in Ruby as possible, it also prevents the actual number values from being copied every single time they are passed from V8 to Ruby. Instead, there is just one copy in the V8 heap until it is deallocated.

Also, in this PR, I've experimented with using c++11 lambda pointers to implement the Ruby methods. I think that it makes the code a lot more readable, and it eliminates something like %25 of the boiler-plate. If this pans out, then it may be worth converting the whole codebase to this style.

cowboyd commented 9 years ago

@stormbreakerbg I removed all of the c++11 features that were being used, so it should be ready for review.

Also, I think I was able to reduce a lot of boilerplate by inlining the functions in the header file.

ignisf commented 9 years ago

Why did you decide on not using c++11?

georgyangelov commented 9 years ago

I can't quite wrap my head around the Integer class. Its New methods return an instance of either Int32 or UInt32, but it also has a Value method.

cowboyd commented 9 years ago

@ignisf We didn't decide to not use c++11, rather, we decided not commit to it yet until we can decide on what constitutes a reasonable support matrix for the systems we want to target.

The concurrent queue that we're using depends on it, so not using c++11 would mean having to eventually find a new implementation or write our own.

That said, If we could pull together a robust binary release, then the issue of what C compiler is on the target becomes much less relevant. I've had bad luck with this in the past, but then, we know how I fare when doing battle with builds.

I'd describe our stance right now as a "holding pattern"

cowboyd commented 9 years ago

@stormbreakerbg Yes, and that Value method returns int64_t, so what gives?

What I take that to mean is that there are times when the runtime will produce values which are too wide for an int32_t or a uint32_t. But there is a) no way to create those values directly, and b) no corresponding IsInteger method to test if that's what you've got.

cowboyd commented 9 years ago

@ignisf Just spitballing here, but how feasible do you think a binary release of therubyracer proper would be?

ignisf commented 9 years ago

DISCLAIMER: I am fuzzy on linking theory & on how the hell do binary gems with dynamically linked libraries even work given the diversity of ruby and dependency ABIs that exists (esp. in the Linux world).

The main issue here is with portability of the source vs portability of the .so vs possibility/practicality of distributing a statically linked version of the library.

The conventional (canonical) way of making a C++ program portable is adding build scripts, distributing it in source form and letting the build scripts and the compiler produce a binary in accordance to the current environment and architecture. What's different in our case though is potentially having source that's incompatible with part of the systems we intend to support, given that they do not have a compiler with C++11 support.

Whenever one searches for information about the portability of a shared library, the general consensus is 'don't do it' which seems to stem from the unpredictability of how this shared library might behave in a different environment. This is worrying having in mind that rubygems is 'orthogonal' to distro-specific packaging systems and providing distro-specific versions of the gem is neither possible, nor practical with it. However, I know that Skype do it for their binaries, and TRR has a lot less deps than Skype. It seems feasible, but I don't have any references to back this with.

Distributing TRR as a statically linked library (or only the C++11-having part) is an interesting concept. I am very fuzzy on the details here though. As far as I've seen, one approach here would be to provide a dynamic wrapper around the statically compiled library. This whole thing however seems questionably practical.

@stormbreakerbg might be able to chime in here

georgyangelov commented 9 years ago

@ignisf I'm sure you are a lot more knowledgeable in the linking stuff than me. I haven't really done any library distribution.

cowboyd commented 9 years ago

We're effectively taking this strategy part of the way by statically linking the libv8 binaries into the dynamic ruby extension, so at some level it is theoretically possible, although the only time I've really heard of this being successful is on Windows with Nokogiri being the exemplar.

We'd probably have to compile one dynamic library for each ruby ABI we'd want to support, which could change at any time and without warning since the Ruby Team is not committed to maintaining any ABI backwards compatibility.

On my mac:

$ otool -L tmp/x86_64-darwin14.0/init/2.1.5/init.bundle                                        ✭ ◼
tmp/x86_64-darwin14.0/init/2.1.5/init.bundle:
    /usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1213.0.0)
    /usr/local/lib/libgmp.10.dylib (compatibility version 13.0.0, current version 13.0.0)
    /usr/lib/libstdc++.6.dylib (compatibility version 7.0.0, current version 104.1.0)

I'm sure there are similar dynamically linked libraries present on linux.

So what you're saying is that theoretically, we could link those 4 libs statically into the dynamic library?

ignisf commented 9 years ago

So what you're saying is that theoretically, we could link those 4 libs statically into the dynamic library?

AFAIK not in any convenient way through mkmf:

https://sourceware.org/binutils/docs/binutils/ar-scripts.html

ADDLIB archive ADDLIB archive (module, module, ... module)

Add all the contents of archive (or, if specified, each named module from archive) to the current archive.

Requires prior use of OPEN or CREATE.

https://gcc.gnu.org/onlinedocs/gcc/Link-Options.html

-static-libstdc++

When the g++ program is used to link a C++ program, it normally automatically links against libstdc++. If libstdc++ is available as a shared library, and the -static option is not used, then this links against the shared version of libstdc++. That is normally fine. However, it is sometimes useful to freeze the version of libstdc++ used by the program without going all the way to a fully static link. The -static-libstdc++ option directs the g++ driver to link libstdc++ statically, without necessarily linking other libraries statically.

ignisf commented 9 years ago

Oh and also http://blog.zachallett.com/howto-ruby-c-extension-with-a-static-library/

cowboyd commented 9 years ago

@stormbreakerbg I played it safe, and return the specific type in the constructor because there were cases where even if you used the unsigned constructor, v8 was storing it internally as an Uint32. No need to go through the full Value downcast, but just do a check to see which it is, and then return that specific subtype.

georgyangelov commented 9 years ago

Ok :+1: