rust-js / rjs Goto Github PK

JsValue has a number of cast methods that unwrap to the underlying value. These should be renamed. The reason for this is that these methods panic when the value isn't of the specified type, but the method names aren't scary enough.

Renaming should be done as follows:

• For a method like to_bool;
• Should be renamed to unwrap_bool; and
• A second method should be introduced named unwrap_bool_or that takes a default value for when the value isn't of the specified type.

The LoadGlobal and StoreGlobal instructions could be replaced with LoadEnv and StoreEnv.

Function creation in bootstrapping may be a bit more elegant. There is some code duplication there to get object, function and array setup correctly. See whether the duplication can be limited.

http://www.cs.ucla.edu/~palsberg/course/cs132/linearscan.pdf describes a fast algorithm for implementing a register allocator, specifically suited for JIT's.

For the ECMA tests, every time a script is run, the output is added to the debug output. This creates a lot of noise. Instead the debug output should be reset just before the actual test script is run.

The pointer fields of the garbage collector types should be a Unique instead of a few pointer.

There are a number of places where root handles need to be converted to local handles. This specifically applies to JsEnv.global and JsEnv.global_scope. It should be possible to prevent this. I've started work on implementing a trait over both Local and Root but bumped into some issues. This should be picked up at a later time.

JsDescriptor is a very large and often use struct. This struct can be greatly optimized, e.g. by using bitmaps.

There are a lot of match constructs that match on reference and have the &... => construct in the arms. This can be rewritten to matching on the derefed value instead.

The hash and array store currently use the same backing storage. Instead they should both use an optimized storage format. At least we should move the accessor out of the entries and to a heap object. This optimizes for simple values.

Deopt mode (which kicks in when eval come in to play) significantly change the working of the interpreter. To thoroughly test this functionality a mode should be added that forces everything into deopt mode. This way all ECMA tests can be used to verify the correctness of the implementation. This feature should be available for all modes (so interpreted, jitted and optimized) and should be part of the standard test matrix.

Test built-ins/Date/15.9.1.15-1.js tests for a specific ISO date parsing. This test is currently disabled, but it must be checked whether this is correct.

The core data structures (like JsObject and HashStore) need to be optimized for size. One of the changes is that #[repr] needs to be added to them to guarantee the memory layout.

Note that JsFn is 16 bytes and should be looked at. Maybe adding #[repr] here fixes that.

When a lifted variable come from function more than one deep and the intermediate function does not have lifted variable, an empty scope is created. This can be elided.

Currently the arg method on JsArgs is returning a Local<JsValue>. If the requested index is not in the arguments list, it returns a env.new_undefined(). To work with this, JsArgs also implements a arg_or, a map_or and a map_or_else, all of which is available on Option<T>.

Instead, arg should be returning an Option<Local<JsValue>> or even Option<JsValue> so that the unwrap and map methods on Option<T> can be used.

The Name enum represents both interned strings and indexes. The reason for this is that this simplifies the implementation (we don't need separate methods for indexed access and checks on keys being an index if it's not necessary, e.g. for JsObject). However, a result of this is that index keys (positive numeric keys that are less or equal to i32::MAX) are not interned anymore. Instead we create a new String when the string for such a Name is asked from the interner. This should not be that much of a problem because it is expected that there will be very few such keys in normal usage. However, this must be validated. When doing performance tests, a metric should be made available how many times such a string was created. If it turns out that this assumption does not hold, we may need to revisit this design decision.

The current garbage collector implementation is far from ideal. The LuaJIT garbage collector looks like it would be relatively easy to implement. See https://github.com/LuaDist/luajit/blob/master/src/lj_gc.c for the code for this.

Objects currently only support a hash based backing storage. Schema based storage should be implemented too.

The idea of schema based storage is that storage is separated into two parts. The backing storage for the object itself becomes a simple array. Indexes into this array are determined by a schema. This schema is a hash that maps names to indexes.

Every time a new key is added to the object, a new schema is created. This new schema is a copy of the previous schema that maps the new name to the end of the array. These two schemas are then connected to each other using a migration step, so:

• Every object starts out with an empty map, named schema0;
• Key a is added to the map. This creates schema1 that maps a to index 0, and a migration that says to go from schema0 to schema1 when a is added. This schema is then attached to the instance that had a added;
• Key b is added to the map. This creates schema2 that maps b to index 1, and a migration that says to go from schema1 to schema2 when b is added. This schema is then attached ot the instance that had b added.

When either a key is deleted, or the array becomes too large (must be determined based on performance tests), the backing storage for the object switches to a hash based store (current implementation). To make backing storage for instances even smaller, we could also state that we also automatically switch to classic storage when either an accessor is added or when either of the properties (writable, configurable, enumerable) is set to false.

This scheme has two primary advantages:

• Objects require (far) less storage;
• Certain optimizations become a lot simpler. The JIT could depend on this scheme and hard code schema ID's into the code, optimizing access to simple array access when the schema equals some known schema.

See e.g. http://jayconrod.com/posts/52/a-tour-of-v8-object-representation for more information on this.

Arguments are currently loaded from an arguments Vec passed to the interpreter. This must be changed to loading from the stack. If the call is from a native javascript function the variables are already on the stack and can be loaded from the. Otherwise the arguments need to be pushed into the stack before the call is done.

The typeof keyword could be lowered to specialized instructions. If the parser finds a construct identifier = typeof "type" with a known string, the instruction could be lowered into a specialized instruction that specifically checks for that type.

The JsFn type, the type of native functions, must become 'static to enforce it's a free standing function.

Programs and eval's return the result of the last executed expression. Currently this is implemented by requiring all expression statements to put the result in a local for the last statement of a program. This recurses into the statement, so if a statement contains other statements, those expression statements apply too. Currently, all expression statements in that tree will write their result into a local. However, this is not required. Only the last statement of a block and the last statement before break statements should be writing their results.

The get_value method is in the ECMA specs but does not apply to the rjs implementation. Remove it.

There are a number of flags that influence compilation and execution. These are strict, JsFnMode and the compilation flags (strict and ScopeType). Review all of these. For the compilation flags, simplify the flags that are actually provided to build_ir and work back from there. For the rest, see what we can do without and can be removed.

Because of the change to stack passing of arguments, a number of local scopes can be removed.

There are a number of tests that seem to be a spec violation. All ignored tests are also tests that fail on at least Google Chrome and Firefox. See references to this bug in the test262-ignore.json file for the applicable tests.

Research must be done into whether this interpretation is correct and whether these tests can safely be ignored.

It looks like [[Put]] is implemented wrong. It seems that the following should allow the array item to be changed:

Object.defineProperty(Array.prototype, "0", {
    value: 100,
    writable: false,
    configurable: true
});

oldArr = [];
oldArr[0] = 102;

At least this works in Chrome and Internet Explorer (i.e. oldArr[0] returns 102). However I cannot find where in the specs this is allowed.

put currently has a special case that checks for putting an index into an array and skips the can_put check if this is the case.

The default of RustDT is to use tabs as code indentation. However the standard for Rust is to use spaces.

The f64 postfix can be replaced with either a .0 or exponent format.

-1 is parsed as neg 1.

Calls to is_nan and is_infinite can be combined into a single is_finite call.

All GC structs should be made #[repr].

There are a lot of relative use super::... statements. This should be rewritten to absolute use statements.

The LocalScope functionality implements support for scoped rooted objects. The current implementation is not performant enough. One way to go is to pool backing storage, e.g. using (something like) https://github.com/frankmcsherry/recycler.

There are a lot *const c_void types. If the size of *(*const ()) is also 1, that should be used instead.

The function constructor allows functions to be created from strings. The current implementation is not ideal and should probably have more direct hooks into the parser.

The Date implementation is a bit of a mess. It passes the tests, but is too much a verbatim implementation of the specs. The existing code should be refactored and cleaned up.

The function scope object is loaded too early. This should be delayed until we're in the interpreter.

All variables that are passed out of managed code are accessed as Local references. This creates a heavy performance hit. A way to work around this is to instead of working with Local's, optimize for not having to need them. The idea is that probably the garbage collector won't run when outside of managed code. If the garbage collector would run, instead of performing a run, allocate a temporary block of memory and allocate from there. When we come out of native code, check for such blocks, perform a collection and merge these blocks into the main garbage collected memory.

It looks like nested local scopes are not rooted. When the current local scope is at level 1, the scope at level 0 may not be taken into account as roots.

The string and scope objects are simple arrays. Currently the objects themselves contain a single Array pointer. These pointers should be inlined into the JsValue, saving a pointer chase and heap allocation.

GC types that don't have pointers should be skipped completely. The current walker implementation marks then as Next. However just returning Endis not enough since the garbage collector will still walk over all array entries. Instead a new return code should be added that signifies that the type will never contain pointers and that array walking should be halted. This e.g. applies to strings.

At the moment, call and construct take a Vec. This means that we need to create a lot of locals. Instead it should be possible to put all of this on the stack for optimized calls. The current interface should only really be used when calling functions and constructors via the public API. The internal API should use an optimized calling convention.

The isize and usize data types are used a lot. In most of these places, this is OK. However, there are a number of them that should be reviewed, specifically:

• Name should use u32 for the index type. Using usize for the name type is fine;
• Casting to isize or usize, e.g. in env/array.rs should be limited and use i32 and u32 instead. Or, the values should be lowered from f64 later.

Memory allocation in the garbage collector currently only supports Windows. See https://github.com/jemalloc/jemalloc/blob/ef0a0cc3283ea561a40b33f4325d54bbc351de21/src/chunk_mmap.c for how this can be implemented for Linux.

assert!'s are also emitted in release builds. Switch to debug_assert! where possible.

The ECMA 262 test suite contains hard coded results for Math.pow. These numbers differ from what we produce. The results must be verified to make sure it's OK we don't conform to the test results.

These tests currently are ignored in test262-ignore.json. Check that file for references to this issue.

Currently AstVisitor return (). In the local visitor, this requires us to keep Booleans for syntax errors. AstVisitor should be made to return JsResult so we can throw proper errors.

Floating point parsing and printing is very limited in Rust. Parsing is just plain wrong, see e.g. Rust issue #24557, and printing can only be done using static formatters using the format! macro.

The following should output 6, not 7:

function f(a) {
    arguments[0]++;
    with ({a:1}) {
        var g = function (b) {
            arguments[0]++;
            return a + b + eval('1');
        }
    }
    return g(a + 1);
}
throw f(1);

The problem is that a is resolved to the argument and is not dynamically resolved to the with local.