Un-template various public GC tracing/marking functions
Categories
(Core :: JavaScript Engine, task, P1)
Tracking
()
People
(Reporter: Waldo, Assigned: Waldo)
Details
Attachments
(4 files)
Bug 1426865 alleges that having JS_PUBLIC_API template functions seems to force additional publicity on pointer-typed symbols that appear in their arguments. Or something. Moreover, having public template functions that are defined and instantiated only in .cpp files inside SpiderMonkey, means it's hard/impossible to see what types are used to instantiate the template.
These template functions -- where they are not defined as inlines -- should be converted to just plain old JS_PUBLIC_API overloads of a single function name. (And once that's done, it should be easier to separate out the purely-internal GC pointer types and their handling to differently-named overloads, so that we don't have those bits polluting public API.)
(The end goal of all of this is to stop having JS_PUBLIC_API on classes in the public API that are only used by pointer and are basically abstract. By putting JS_PUBLIC_API on those classes, we seem to be compelling a massive number of very-internal functions to be exported, for no reason and probably with deleterious effects with respect to linking, inlining, and so on. But this change is just a start, and there'll still be more to do after this is done, for sure.)
|
Assignee | |
Comment 1•4 years ago
|
||
|
||
Updated•4 years ago
|
|
|
Assignee | |
Comment 2•4 years ago
|
||
Depends on D56804
|
|
Assignee | |
Comment 3•4 years ago
|
||
Depends on D56805
|
|
Assignee | |
Comment 4•4 years ago
|
||
Depends on D56806
|
||
Updated•4 years ago
|
|
||
Comment 5•4 years ago
|
||
Jon, I'm fine with reviewing this, but I'd be interested to hear your thoughts. Unless I'm missing something, these changes seem strictly better. And they might also trim down the number of instantiations of various template functions. Which is good for code size and good for compile and link times. But I also know that I repeatedly run into problems based on what is or isn't visible from various compilation units. I think everything here is safe in that respect. (The usual problem is that I want a fallback instantiation based on information in the full types, and some compilation don't have the full types and so incorrectly choose the fallback. But I don't believe that applies here.)
|
|
Assignee | |
Updated•4 years ago
|
Pushed by jwalden@mit.edu: https://hg.mozilla.org/integration/autoland/rev/384c1998a982 Move the GC pointer type enumeration macros to a standalone header. r=sfink
|
|
Assignee | |
Comment 7•4 years ago
|
||
Oh, FWIW as to these functions -- it may be worth noting that because they're taking T**, and U** does not implicitly convert to T** even if U converts to T, there's no worry about this change causing a function to be accidentally mis-invoked in these changes.
|
||
Comment 8•4 years ago
|
||
| bugherder | ||
|
||
Comment 9•4 years ago
|
||
(In reply to Steve Fink [:sfink] [:s:] from comment #5)
I like this approach because it restricts the API to the specific types we want to allow. So this LGTM.
|
||
Comment 10•4 years ago
|
||
Pushed by jwalden@mit.edu: https://hg.mozilla.org/integration/autoland/rev/240c74a53e88 Define |js::UnsafeTraceManuallyBarrieredEdge| as not a template function, and only for the single |JSObject*| GC type that needs it. r=sfink https://hg.mozilla.org/integration/autoland/rev/0de337062155 Convert |js::gc::TraceExternalEdge| from a template to separate function overloads. r=sfink https://hg.mozilla.org/integration/autoland/rev/e68519117ace Convert |JS::UnsafeTraceRoot| from a template to separate function overloads. r=sfink
|
||
Comment 11•4 years ago
|
||
| bugherder | ||
|
|
Assignee | |
Updated•4 years ago
|
Description
•