Write barrier support was added to the JSC runtime in anticipation of the implementation of more sophisticated garbage collection schemes, such as possibly generational or incremental collection. Write barrier fast paths can often be optimized for specific kinds of writes. In some collectors, a store to a location that is in the heap can be optimized far more aggressively than a store to a location allocated outside the heap. One example of this is the range-based generational barrier, which uses the heap address of the destination object as a quick check to see if the barrier needs to be run. But designing such barriers requires knowing what kinds of writes programs tend to execute. Currently, there is no way to tell the barrier what kind of write is being executed. And there is no way to profile and report the kinds of writes encountered.
Created attachment 104796 [details] the patch
Created attachment 104797 [details] the patch (fix conflict with my earlier patch)
Comment on attachment 104797 [details] the patch (fix conflict with my earlier patch) Attachment 104797 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/9475286
Created attachment 104801 [details] the patch (fix added file) In the previous patch, I forgot to include a file I had added.
Comment on attachment 104801 [details] the patch (fix added file) View in context: https://bugs.webkit.org/attachment.cgi?id=104801&action=review Looks good, but I think the C++ counter is not quite right. > Source/JavaScriptCore/runtime/WriteBarrier.h:151 > // when some basic types aren't yet completely instantiated > void setEarlyValue(JSGlobalData&, const JSCell* owner, T* value) > { > +#if ENABLE(WRITE_BARRIER_PROFILING) > + WriteBarrierCounters::usesWithBarrierFromCpp.count(); > +#endif > this->m_cell = reinterpret_cast<JSCell*>(value); > Heap::writeBarrier(owner, this->m_cell); I think you want to put the counter inside Heap::writeBarrier, and not setEarlyValue. Heap::writeBarrier is the central funnel that will catch all C++ access.
Created attachment 104902 [details] the patch (fix review)
Comment on attachment 104902 [details] the patch (fix review) View in context: https://bugs.webkit.org/attachment.cgi?id=104902&action=review r=me, but you have a minor bug here in the ENABLE(GGC) case to fix. > Source/JavaScriptCore/heap/Heap.h:253 > inline void Heap::writeBarrier(const JSCell* owner, JSValue value) > { > + WriteBarrierCounters::countWriteBarrier(); > if (!value) > return; > if (!value.isCell()) Since this version of Heap::writeBarrier calls through to the JSCell* version, you'll double-count cases where you don't early return. You need to call WriteBarrierCounters::countWriteBarrier() inside each early return, and not in the body of the function.
Created attachment 104969 [details] the patch (fix review)
Attachment 104969 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/heap/Heap.h:135: The parameter name "cell" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 104969 [details] the patch (fix review) Oops, apparently the fix breaks style.
Created attachment 104970 [details] the patch (fix style)
Comment on attachment 104970 [details] the patch (fix style) Clearing flags on attachment: 104970 Committed r93698: <http://trac.webkit.org/changeset/93698>
All reviewed patches have been landed. Closing bug.
I think that a better nickname for “calls from C++ as opposed to from JavaScript” would be FromC rather than FromCpp. I find it hard to parse Cpp since it’s not capitalized as an acronym and when used as an acronym it often is used to mean “C preprocessor”. I think that C is a fine nickname for “the C family including C++” in this context.