Especially when ropes are being used in anger, a lot of JSRopeString objects are never resolved. Thus they don't need destruction. Since destruction is expensive, that would be useful to know. Ropes are usually unresolved because people do something like this: var result = ""; for (things) result += stuff use result In that case, we will create a JSRopeString for each iteration of the loop, but we will only resolve the last one. This comes up over and over again. For example, this: "foo" + bar + "baz" + fuzz + "buzz" + thingy + "stuff" will create three JSRopeStrings, but only the last one will get resolved.
Created attachment 341234 [details] work in progress
Created attachment 341338 [details] maybe it'll work
This patch has a crash bug, it's not any faster, and it wastes a lot of memory. I'm investigating. :-/
Created attachment 341361 [details] seems to kinda work But it's not a speed-up.
(In reply to Filip Pizlo from comment #4) > Created attachment 341361 [details] > seems to kinda work > > But it's not a speed-up. Or, rather, it's a *very small* speed-up. I'll play with it a bit more before giving up.
Seems to not be super great for perf. If anything, JetStream might be slower. I think that part of the problem is that those blocks that have unresolved rope strings tend to also not be completely empty, so we have to walk over them anyway. The high cost of creating ropes might also be so great as to dominate over the benefit of reducing sweep cost.
Created attachment 341413 [details] the patch Tiny splay speed-up. The speed-up is minuscule - around 1% - but significant. I confirmed it in run-jsc-benchmarks, in-browser JetStream, and running the benchmark manually. It could be an even bigger speed-up if I made bump'n'pop's pop path faster.
Attachment 341413 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSRopeStringHeapCellType.cpp:30: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 341413 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=341413&action=review r=me > Source/JavaScriptCore/heap/MarkedBlock.cpp:460 > +void MarkedBlock::Handle::sweep(FreeList* freeList) Replacing the position of `MarkedBlock::Handle::sweep` and `MarkedBlock::Handle::finishSweepWithNoDestructors` would make diff clean.