JavaScript:SpiderMonkey:Technical Debt

From MozillaWiki
Jump to: navigation, search
Ambox outdated.png THIS PAGE MAY BE OUTDATED
This article is in parts, or in its entirety, outdated. Hence, the information presented on this page may be incorrect, and should be treated with due caution until this flag has been lifted. Help by editing the article, or discuss its contents on the talk page.

This is a list/overview of the various known sources of technical debt in SpiderMonkey, curated for easy reference. Categorization is inexact, given that issues may span multiple categories.

Garbage collection

Hash table rooting
Using hash tables mapping different kinds of GC things as keys/values is tricky, in the presence of moving GC. We need to do some work, perhaps with templates, to make it easier to safely create and work with such hash tables.
Too many rooting mechanisms!
The story for how to root things in the heap remains more complicated than it should be.

Frontend

Parse nodes are large, public unions
ParseNode::pn_u is largely publicly exposed to everyone, and it's easy to use the wrong union arm for the parse node kind. Potentially this should be converted to a more normal C++ inheritance hierarchy. (But note that we mutate parse node kinds in various places, destroying the "static" nature of a ParseNode's instantaneous subclass as would be reflected in a C++ type.) This will become simpler to address (perhaps by making the sub-structs in pn_u each private, with friendships to ParseNode subclasses) after bug 1130811 is fixed.
Single-pass parsing intertwined with name analysis
The JS parser attempts to do name analysis as it goes, with various bits smuggled into bytecode emission and so on. It really should be moved into a separate pass over the constructed AST.
Emit bytecode in a separate pass, not at the end of each function/script (?).

Meta-object protocol

SpiderMonkey-specific property lookup APIs don't exist in ES6/etc.
The lookupProperty MOP operation isn't in ES6. It's mostly removed, with the exception of template <bool> FetchName(...) operation in the interpreter. Property-setting also contains various weird cases, some for SpiderMonkey-specific gunk and other reasons -- although most of this has been removed, courtesy of jorendorff.
We have both PropDesc and PropertyDescriptor
These two classes are relatively similar and relatively interconvertible -- it shouldn't be much work to convert all cases to use only one of these, now.

Error reporting

General API uncleanliness
Error numbers encoding error type isn't very pleasant: it's not possible to see, at a throw-site, what sort of error is thrown. The number of parameters passed to throw an error is overlarge. There are a bunch of error-reporting methods, rather than a small, constrained set. Generally there should be a much smaller API surface. (It would also be nice if error reporting could be localized, such that localized error messages could appear in the console, although probably not in error messages as observable by scripts.)
Error reporting mechanisms live in JSContext, not in JSRuntime (perhaps in a sub-struct within it)
Error reporting should be runtime-based, as JSContext is increasingly irrelevant as a concept.

JSAPI

JSNative should take const CallArgs& or CallArgs
Essentially all such functions immediately create a CallArgs internally and work only with that, eschewing use of argc and vp. It would be better to cut out the middlemen, if we can do so respecting ABI in the JITs.
Remove JSContext by moving everything in it into JSRuntime
Certain places in Gecko want to have different behavior -- different error objects, primarily -- when different contexts are used. (See the item in the Error Reporting section.) Once that's fixed, JSContext as an independent concept, and part of API, can probably be removed.

JS shell

General cleanliness issues
The shell isn't exactly the most pristine example of JSAPI usage.

JIT

Overlarge files
Certain files -- js/src/jit/IonBuilder.cpp, js/src/jit/MIR.h, js/src/jit/CodeGenerator.cpp, various macro assembler files -- are quite large. It might be better to split some of these up to some degree. There are some bugs on file for this (sunfish's mentored bug?).
Define the MacroAssembler interface in a single header
It's very difficult to know what's in platform-specific macro assemblers, and what's in all macro assemblers. Putting the interface for all platforms into a single struct in a single header would greatly aid readability/clarity as to what's usable in what backends.

Standard library

Builtin implementations shouldn't reside in js/src/js*.*
Builtins should be implemented in js/src/builtin.

VM

VM implementation shouldn't reside in js/src/js*.*
VM details should be in specific files within js/src/vm.
Object layout (JSFunction stores extra stuff one way, ArrayBuffers another, maybe Servo stuff using a third, SharedArrayBuffer wanting atomic stuffs for a fourth?)
can we create a unified system for this?

Documentation

Move documentation out of MDN, into the tree, in ReadTheDocs formattting
Our docs are more likely to remain up-to-date if the documentation is immediately adjacent to the relevant code
Various large-scale things aren't documented at all, even on MDN
This is particularly a problem for things that don't get driveby attention.

General

Kill off the remaining js*.* files
Generally, we want to make js/src contain pretty much only other folders.
Add a js/src/README.txt describing the overall organization of SpiderMonkey code
Self-explanatory.
Error reporting is a disaster.
JS_ReportError is a bad API that doesn't indicate the type of the error thrown, at the location the error's thrown. We need new stuff here. Also, we have two separate manners for tracking errors: exceptions, and error reporting. We should have only one. It would also be nice if error throwing produced a stack even if an error object isn't created/thrown at that spot.