It would be much better to have GC pointer and similar annotations embedded in the source code rather than an external annotations.js file.

This patch actually adds the tracked annotations to the xdb files for use by the analysis.

Ok, finally got around to trying to land this. I will also r? you on the sixgill changes, but note that they are already deployed. ;-) And my apologies for folding together multiple changes, but the test and the annotation stuff are heavily interrelated, and the base class stuff is minor and would just be some extra effort to separate out.

Comment on attachment 8717209 [details] [diff] [review] Record and track __attribute__((tag("Foo"))) annotations to allow annotations to be embedded in source code On second thought, I think I'll give these reviews to bhackett, since they deal with a bunch of fiddly sixgill-specific stuff that I'm pretty shaky on, and Brian will be able to easily recognize what's irrelevant boilerplate (and tell me what I'm doing horribly wrong.)

Comment on attachment 8729734 [details] [diff] [review] Switch to using in-source annotations. Use C++ inheritance information when describing GC types. Add a test suite Review of attachment 8729734 [details] [diff] [review]: ----------------------------------------------------------------- The python test runner could use some work, but that can come later. Ship it! ::: js/src/devtools/rootAnalysis/annotations.js @@ -403,5 @@ > -// seems useful for something like /Vector.*Something/. > -function isGCPointer(typeName) > -{ > - return false; > -} \o/ ::: js/src/devtools/rootAnalysis/computeCallgraph.js @@ +218,5 @@ > +function getTags(functionName, body) { > + var tags = new Set(); > + var annotations = getAnnotations(body); > + print(functionName); > + print(JSON.stringify(annotations)); Are these debugging prints intentional? @@ +236,5 @@ > if (!('PEdge' in body)) > return; > > + for (var tag of getTags(functionName, body).values()) > + print("T " + memo(functionName) + " " + tag); Is this debugging code intentional? ::: js/src/devtools/rootAnalysis/computeGCTypes.js @@ +207,5 @@ > markGCType(typeName, '<pointer-annotation>', '(annotation)', 1, 0, ""); > } > > +//for (var type of listNonGCPointers()) > +// annotations.NonGCPointers[type] = true; I guess this can go now. @@ +223,4 @@ > return; > } > if (fields.has('<pointer-annotation>')) { > + print(indent + "which is annotated as a GCPointer"); \o/ ::: js/src/devtools/rootAnalysis/run-test.py @@ +1,1 @@ > +import sys I guess this isn't /quite/ test code, so might as well include a license header. ::: testing/mozharness/scripts/spidermonkey/build.shell @@ -4,5 @@ > set -x > > [ -d $ANALYZED_OBJDIR ] || mkdir $ANALYZED_OBJDIR > cd $ANALYZED_OBJDIR > -$SOURCE/js/src/configure --enable-debug --enable-optimize --enable-stdcxx-compat --enable-ctypes --enable-exact-rooting --enable-gcgenerational --with-system-nspr Heh.

Comment on attachment 8717209 [details] [diff] [review] Record and track __attribute__((tag("Foo"))) annotations to allow annotations to be embedded in source code Review of attachment 8717209 [details] [diff] [review]: ----------------------------------------------------------------- ::: imlang/type.cpp @@ +152,5 @@ > for (size_t aind = 0; aind < ny->GetArgumentCount(); aind++) > Type::Write(buf, ny->GetArgumentType(aind)); > WriteCloseTag(buf, TAG_TypeFunctionArguments); > } > + if (ny->GetAnnotationCount() > 0) { This |if| is unnecessary.

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/45a405814365

Oops. I didn't resolve review comments here. (In reply to Terrence Cole [:terrence] from comment #6) > ::: js/src/devtools/rootAnalysis/computeCallgraph.js > @@ +218,5 @@ > > +function getTags(functionName, body) { > > + var tags = new Set(); > > + var annotations = getAnnotations(body); > > + print(functionName); > > + print(JSON.stringify(annotations)); > > Are these debugging prints intentional? No. Darn, I'll have to strip them out. > @@ +236,5 @@ > > if (!('PEdge' in body)) > > return; > > > > + for (var tag of getTags(functionName, body).values()) > > + print("T " + memo(functionName) + " " + tag); > > Is this debugging code intentional? That isn't debugging code, it's a space station. Er, I mean, it's the output that communicates tags to the next stage of processing. > ::: js/src/devtools/rootAnalysis/computeGCTypes.js > @@ +207,5 @@ > > markGCType(typeName, '<pointer-annotation>', '(annotation)', 1, 0, ""); > > } > > > > +//for (var type of listNonGCPointers()) > > +// annotations.NonGCPointers[type] = true; > > I guess this can go now. Er... actually, I don't see where I added an in-source annotation for that one. I'm not sure I can, since NPIdentifier (now the only thing in listNonGCPointers()) is a typedef for void*, and I don't think we can see it. Then again, this isn't erroring out, so I guess we don't have any hazards with NPIdentifier anyway? > ::: js/src/devtools/rootAnalysis/run-test.py > @@ +1,1 @@ > > +import sys > > I guess this isn't /quite/ test code, so might as well include a license > header. Ok. I'll add one to js/src/tests/jstests.py while I'm at it.

https://hg.mozilla.org/mozilla-central/rev/74feb4250db0 https://hg.mozilla.org/mozilla-central/rev/09a8f6c7767e