JSDollarVMPrototype::isValidCell() uses a heap object iterator to validate candidate cell pointers, and, when in use, is called a lot more often than the normal way those iterators are used. As a result, I see my instrumented VM killed with a SIGXCPU (CPU time limit exceeded). This patch gives the callback functor the ability to tell the iterators to return early when the functor no longer needs to continue iterating. With this, my instrumented VM is useful again for debugging. Since heap iteration is not something that we do in a typical fast path, I don’t expect this to have any noticeable impact on performance.
Created attachment 251261 [details] the patch.
Comment on attachment 251261 [details] the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=251261&action=review > Source/JavaScriptCore/heap/Heap.cpp:1453 > + return false; // Not done iterating. If an API requires a comment at every usage site, that's a good sign that the API is not readable by design.
(In reply to comment #2) > Comment on attachment 251261 [details] > the patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=251261&action=review > > > Source/JavaScriptCore/heap/Heap.cpp:1453 > > + return false; // Not done iterating. > > If an API requires a comment at every usage site, that's a good sign that > the API is not readable by design. I'm open to suggestions.
Comment on attachment 251261 [details] the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=251261&action=review >>> Source/JavaScriptCore/heap/Heap.cpp:1453 >>> + return false; // Not done iterating. >> >> If an API requires a comment at every usage site, that's a good sign that the API is not readable by design. > > I'm open to suggestions. Make it an enum.
(In reply to comment #4) > Comment on attachment 251261 [details] > the patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=251261&action=review > > >>> Source/JavaScriptCore/heap/Heap.cpp:1453 > >>> + return false; // Not done iterating. > >> > >> If an API requires a comment at every usage site, that's a good sign that the API is not readable by design. > > > > I'm open to suggestions. > > Make it an enum. Sounds like a good plan. I will execute.
Created attachment 251276 [details] patch 2: no more comments.
Comment on attachment 251276 [details] patch 2: no more comments. View in context: https://bugs.webkit.org/attachment.cgi?id=251276&action=review > Source/JavaScriptCore/ChangeLog:17 > + I donât expect this to have any noticeable impact on performance. Will fix the apostrophe in “don’t” before I land.
Comment on attachment 251276 [details] patch 2: no more comments. r=me
Thanks for the review. Landed in r183124: <http://trac.webkit.org/r183124>.