RESOLVED FIXED 142510
Web Inspector: Include Garbage Collection Event in Timeline
https://bugs.webkit.org/show_bug.cgi?id=142510
Summary Web Inspector: Include Garbage Collection Event in Timeline
Tobias Bosch
Reported 2015-03-09 15:19:51 PDT
The Angular team is creating a benchmark tool, which reads out the inspector timeline data to get detailed numbers about script time, rendering time as well as garbage collection time/amount. To get stable numbers we are subtracting the gc time from script execution time. However, GCEvents are no more reported in newer versions of Safari (see commits below). Is there a chance of getting them back? Also, showing them in the inspector UI would be nice for regular web developers as well. Btw, is there a way to force gc in Safari / mobile Safari? We do this in Chrome and it helps to stabilize our benchmark numbers further. Relevant commits: - File https://github.com/WebKit/webkit/blob/master/Source/WebCore/inspector/InspectorTimelineAgent.cpp, - Commit d5b7f4f8f0956a2b0c05162ac1a25287a8c9c74c removed reporting GCEvents and switched to adding a `usedHeapSize` property to all records - Commit 7fe486eb595c650aad97401aebc5086257cae8c4 removed gc reporting alltogether. Thanks a lot, Tobias
Attachments
[PATCH] Proposed Fix (77.41 KB, patch)
2015-09-28 17:37 PDT, Joseph Pecoraro
no flags
[IMAGE] GC in Overview (93.19 KB, image/png)
2015-09-28 17:39 PDT, Joseph Pecoraro
no flags
[IMAGE] GC in Scripts & Events (95.55 KB, image/png)
2015-09-28 17:39 PDT, Joseph Pecoraro
no flags
[PATCH] Proposed Fix (77.45 KB, patch)
2015-10-01 19:15 PDT, Joseph Pecoraro
no flags
[PATCH] Proposed Fix (Improve Test, Fix Builds) (78.86 KB, patch)
2015-10-01 19:36 PDT, Joseph Pecoraro
ggaren: review+
[PATCH] For Bots (79.32 KB, patch)
2015-10-02 11:31 PDT, Joseph Pecoraro
no flags
[PATCH] Proposed Fix (79.94 KB, patch)
2015-10-14 18:54 PDT, Joseph Pecoraro
bburg: review+
bburg: commit-queue-
[PATCH] For Landing (79.91 KB, patch)
2015-10-15 11:46 PDT, Joseph Pecoraro
commit-queue: commit-queue-
Joseph Pecoraro
Comment 1 2015-09-28 17:17:16 PDT
Retitling. Include Garbage Collection (GC) information in the Inspector Timeline.
Radar WebKit Bug Importer
Comment 2 2015-09-28 17:18:38 PDT
Joseph Pecoraro
Comment 3 2015-09-28 17:37:28 PDT
Created attachment 262032 [details] [PATCH] Proposed Fix
Joseph Pecoraro
Comment 4 2015-09-28 17:39:25 PDT
Created attachment 262033 [details] [IMAGE] GC in Overview
Joseph Pecoraro
Comment 5 2015-09-28 17:39:48 PDT
Created attachment 262034 [details] [IMAGE] GC in Scripts & Events
Tobias Bosch
Comment 6 2015-09-28 19:58:32 PDT
Nice, this looks promising! Two questions: 1. In which mobile safari version will this be included? In IOS 9.0 already? 2. Would it be even possible to report the gcAmount again in these events? Thanks a lot for working on this!
Joseph Pecoraro
Comment 7 2015-09-28 21:10:04 PDT
(In reply to comment #6) > Nice, this looks promising! > Two questions: > > 1. In which mobile safari version will this be included? In IOS 9.0 already? I can't comment on future versions of Safari. > 2. Would it be even possible to report the gcAmount again in these events? Yes, I'm looking into including the number of bytes freed by the collection. That is currently a FIXME in the patch above.
Timothy Hatcher
Comment 8 2015-09-28 21:46:45 PDT
iOS 9 has already shipped, so it would not have this change that was just posted.
Joseph Pecoraro
Comment 9 2015-10-01 19:15:39 PDT
Created attachment 262312 [details] [PATCH] Proposed Fix Rebaselined.
Joseph Pecoraro
Comment 10 2015-10-01 19:36:05 PDT
Created attachment 262313 [details] [PATCH] Proposed Fix (Improve Test, Fix Builds)
Geoffrey Garen
Comment 11 2015-10-01 20:57:54 PDT
Comment on attachment 262313 [details] [PATCH] Proposed Fix (Improve Test, Fix Builds) View in context: https://bugs.webkit.org/attachment.cgi?id=262313&action=review r=me > Source/JavaScriptCore/heap/Heap.cpp:1176 > + for (auto* observer : m_observers) I think WebKit usually calls this "client" rather than "observer". > Source/JavaScriptCore/heap/Heap.cpp:1326 > + HeapOperation operation = m_operationInProgress; Revert this line?
Joseph Pecoraro
Comment 12 2015-10-02 11:04:21 PDT
Comment on attachment 262313 [details] [PATCH] Proposed Fix (Improve Test, Fix Builds) View in context: https://bugs.webkit.org/attachment.cgi?id=262313&action=review >> Source/JavaScriptCore/heap/Heap.cpp:1326 >> + HeapOperation operation = m_operationInProgress; > > Revert this line? It is actually needed. We stash the operation before the member gets reassigned to NoOperation below (line 1348). Waiting until after the operation was reassigned was necessary because (in WK1) sending the event to the frontend means evaluating javascript in the frontend page. That would trigger an ASSERT if there was a GC HeapOperation in progress.
Joseph Pecoraro
Comment 13 2015-10-02 11:31:20 PDT
Created attachment 262344 [details] [PATCH] For Bots
WebKit Commit Bot
Comment 14 2015-10-02 13:41:15 PDT
Comment on attachment 262344 [details] [PATCH] For Bots Clearing flags on attachment: 262344 Committed r190520: <http://trac.webkit.org/changeset/190520>
Joseph Pecoraro
Comment 15 2015-10-02 15:30:20 PDT
Rolled out in <http://trac.webkit.org/changeset/190527>. A few tests crashed / asserted.
Joseph Pecoraro
Comment 16 2015-10-02 15:43:35 PDT
Failures were: https://build.webkit.org/results/Apple%20El%20Capitan%20Release%20WK1%20(Tests)/r190523%20(81)/results.html On initial inspection they don't seem like they were related to this patch. WebKit 1 Debug. Looks like forcing a GC via GCController.garbageCollectNow in a test is happening while in the middle of running JavaScript. This seems not directly related to this new change but would be worth fixing. > Exception Type: EXC_BAD_ACCESS (SIGSEGV) > Exception Codes: KERN_INVALID_ADDRESS at 0x00000000bbadbeef > Exception Note: EXC_CORPSE_NOTIFY > > VM Regions Near 0xbbadbeef: > --> > __TEXT 00000001001a8000-00000001001f0000 [ 288K] r-x/rwx SM=COW /Volumes/VOLUME/* > > Thread 0 Crashed:: Dispatch queue: com.apple.main-thread > 0 WTFCrash + 62 (Assertions.cpp:321) > 1 0x100343000 + 5283368 > 2 0x100343000 + 5281771 > 3 JSC::Sweep::ReturnType JSC::MarkedSpace::forEachBlock<JSC::Sweep>(JSC::Sweep&) + 379 (MarkedAllocator.h:140) > 4 JSC::MarkedSpace::sweep() + 41 (MarkedSpace.cpp:120) > 5 JSC::Heap::collectAndSweep(JSC::HeapOperation) + 61 (Heap.cpp:1029) > 6 WebCore::GCController::garbageCollectNow() + 58 (GCController.cpp:89) > 7 collectCallback(OpaqueJSContext const*, OpaqueJSValue*, OpaqueJSValue*, unsigned long, OpaqueJSValue const* const*, OpaqueJSValue const**) + 25 (GCController.cpp:49) > 8 long long JSC::APICallbackFunction::call<JSC::JSCallbackFunction>(JSC::ExecState*) + 571 (APICallbackFunction.h:61) > 9 JSC::LLInt::setUpCall(JSC::ExecState*, JSC::Instruction*, JSC::CodeSpecializationKind, JSC::JSValue, JSC::LLIntCallLinkInfo*) + 592 (LLIntSlowPaths.cpp:1090) > 10 llint_entry + 23124 > 11 vmEntryToJavaScript + 299 > 12 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 158 (JITCode.cpp:81) > 13 JSC::Interpreter::execute(JSC::EvalExecutable*, JSC::ExecState*, JSC::JSValue, JSC::JSScope*) + 1839 (Interpreter.cpp:1270) > 14 JSC::globalFuncEval(JSC::ExecState*) + 1476 (JSGlobalObjectFunctions.cpp:588) > 15 0 + 86571225190440 > 16 llint_entry + 23024 > 17 llint_entry + 23024 > 18 llint_entry + 23024 > 19 vmEntryToJavaScript + 299 > 20 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 158 (JITCode.cpp:81) > 21 JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 447 (Interpreter.cpp:1025) > 22 JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) + 71 (CallData.cpp:45) > 23 WebCore::JSMainThreadExecState::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) + 96 (JSMainThreadExecState.h:56) > 24 Deprecated::ScriptFunctionCall::call(bool&) + 409 (ScriptFunctionCall.cpp:138) > 25 Inspector::InjectedScriptBase::callFunctionWithEvalEnabled(Deprecated::ScriptFunctionCall&, bool&) const + 237 (InjectedScriptBase.cpp:87) > 26 Inspector::InjectedScriptBase::makeCall(Deprecated::ScriptFunctionCall&, WTF::RefPtr<Inspector::InspectorValue>*) + 83 (InjectedScriptBase.cpp:107) > 27 Inspector::InjectedScriptBase::makeEvalCall(WTF::String&, Deprecated::ScriptFunctionCall&, WTF::RefPtr<Inspector::Protocol::Runtime::RemoteObject>*, Inspector::Protocol::OptOutput<bool>*, Inspector::Protocol::OptOutput<int>*) + 55 (InjectedScriptBase.cpp:119) > 28 Inspector::InjectedScript::evaluate(WTF::String&, WTF::String const&, WTF::String const&, bool, bool, bool, bool, WTF::RefPtr<Inspector::Protocol::Runtime::RemoteObject>*, Inspector::Protocol::OptOutput<bool>*, Inspector::Protocol::OptOutput<int>*) + 267 (InjectedScript.cpp:69) > 29 Inspector::InspectorRuntimeAgent::evaluate(WTF::String&, WTF::String const&, WTF::String const*, bool const*, bool const*, int const*, bool const*, bool const*, bool const*, WTF::RefPtr<Inspector::Protocol::Runtime::RemoteObject>&, Inspector::Protocol::OptOutput<bool>*, Inspector::Protocol::OptOutput<int>*) + 329 (InspectorRuntimeAgent.cpp:128) > 30 Inspector::RuntimeBackendDispatcher::evaluate(long, WTF::RefPtr<Inspector::InspectorObject>&&) + 1385 (InspectorBackendDispatchers.cpp:4971) > 31 Inspector::RuntimeBackendDispatcher::dispatch(long, WTF::String const&, WTF::Ref<Inspector::InspectorObject>&&) + 607 (InspectorBackendDispatchers.cpp:4897) > ... WebKit 1 Debug. Enabling the Debugger agent encounters a crash when trying to attach the ScriptDebugServer to a JSGlobalObject. I don't think this patch had made changes here. > Exception Type: EXC_BAD_ACCESS (SIGSEGV) > Exception Codes: KERN_INVALID_ADDRESS at 0x0000000000000012 > Exception Note: EXC_CORPSE_NOTIFY > > Thread 0 Crashed:: Dispatch queue: com.apple.main-thread > 0 JSC::Debugger::attach(JSC::JSGlobalObject*) + 464 (JSCellInlines.h:222) > 1 WebCore::ScriptController::attachDebugger(JSC::Debugger*) + 118 (ScriptController.cpp:328) > 2 WebCore::Page::setDebugger(JSC::Debugger*) + 67 (Page.cpp:1083) > 3 WebCore::PageScriptDebugServer::addListener(Inspector::ScriptDebugListener*) + 71 (PageScriptDebugServer.cpp:70) > 4 Inspector::InspectorDebuggerAgent::enable() + 55 (InspectorDebuggerAgent.cpp:92) > 5 WebCore::WebDebuggerAgent::enable() + 14 (WebDebuggerAgent.cpp:44) > 6 WebCore::PageDebuggerAgent::enable() + 14 (PageDebuggerAgent.cpp:66) > 7 Inspector::DebuggerBackendDispatcher::enable(long, WTF::RefPtr<Inspector::InspectorObject>&&) + 76 (InspectorBackendDispatchers.cpp:2543) > 8 Inspector::DebuggerBackendDispatcher::dispatch(long, WTF::String const&, WTF::Ref<Inspector::InspectorObject>&&) + 607 (InspectorBackendDispatchers.cpp:2527) > 9 Inspector::BackendDispatcher::dispatch(WTF::String const&) + 2467 (Ref.h:55) WebKit 1 Debug. This is a patch made by this patch. After observing a GC there is a crash evaluating JavaScript. Maybe we are doing work too early in Heap::didFinishCollection to be running script now. > Exception Type: EXC_BAD_ACCESS (SIGSEGV) > Exception Codes: KERN_INVALID_ADDRESS at 0x0000000000000018 > Exception Note: EXC_CORPSE_NOTIFY > > Thread 0 Crashed:: Dispatch queue: com.apple.main-thread > 0 0x0000000101839475 JSC::MarkedBlock::FreeList JSC::MarkedBlock::specializedSweep<(JSC::MarkedBlock::BlockState)3, (JSC::MarkedBlock::SweepMode)1, true>() + 181 (MarkedBlock.cpp:95) > 1 0x000000010183910d 0x10132f000 + 5284109 > 2 0x0000000101838bbb 0x10132f000 + 5282747 > 3 0x0000000101837e21 JSC::MarkedAllocator::allocateSlowCase(unsigned long) + 225 (MarkedAllocator.cpp:74) > 4 0x00000001013c9c6f JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&) + 703 (MarkedAllocator.h:94) > 5 0x00000001036566dc WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&) + 284 (ScriptController.cpp:164) > 6 0x0000000103656919 0x102937000 + 13760793 > 7 0x0000000102ee28d4 WebCore::InspectorClient::doDispatchMessageOnFrontendPage(WebCore::Page*, WTF::String const&) + 356 (InspectorClient.cpp:52) > 8 0x00000001016c1fcc Inspector::FrontendRouter::sendEvent(WTF::String const&) const + 44 (InspectorFrontendRouter.cpp:90) > 9 0x00000001016b381d Inspector::HeapFrontendDispatcher::garbageCollected(WTF::RefPtr<Inspector::Protocol::Heap::GarbageCollection>) + 765 (InspectorFrontendDispatchers.cpp:518) > 10 0x00000001016c295a Inspector::InspectorHeapAgent::didGarbageCollect(JSC::HeapOperation) + 730 (InspectorHeapAgent.cpp:122) > 11 0x000000010165babc JSC::Heap::didFinishCollection(double) + 236 (Heap.cpp:1351) > 12 0x000000010165b1cc JSC::Heap::collectImpl(JSC::HeapOperation, void*, void*, int (&) [37]) + 1196 (Heap.cpp:1119) > 13 0x000000010165aced JSC::Heap::collect(JSC::HeapOperation) + 237 (Heap.cpp:1045) > 14 0x0000000101651e28 JSC::GCActivityCallback::doWork() + 72 (GCActivityCallback.cpp:81) > 15 0x00000001016612da JSC::HeapTimer::timerDidFire(__CFRunLoopTimer*, void*) + 186 (HeapTimer.cpp:101) > 16 0x00007fff989f59f4 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 20 > 17 0x00007fff989f5683 __CFRunLoopDoTimer + 1075 > 18 0x00007fff989f51da __CFRunLoopDoTimers + 298 > 19 0x00007fff989ec6d1 __CFRunLoopRun + 1841 > 20 0x00007fff989ebd38 CFRunLoopRunSpecific + 296 > 21 0x00000001011a9e63 runTest(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) + 2146 (DumpRenderTree.mm:2031) > 22 0x00000001011a93ca dumpRenderTree(int, char const**) + 2928 (DumpRenderTree.mm:1288) > 23 0x00000001011aa9da DumpRenderTreeMain(int, char const**) + 1471 (DumpRenderTree.mm:1424) > 24 0x00007fff9a50d5ad start + 1 Tests were also seeing some funky InspectorFrontendAPI not available, and seemingly only with this patch. I don't know what could be causing that! Will be investigating.
Joseph Pecoraro
Comment 17 2015-10-02 15:44:56 PDT
> WebKit 1 Debug. This is a patch made by this patch. Typo: This is a *path* made by this patch.
Joseph Pecoraro
Comment 18 2015-10-02 16:01:39 PDT
Comment on attachment 262344 [details] [PATCH] For Bots View in context: https://bugs.webkit.org/attachment.cgi?id=262344&action=review > Source/JavaScriptCore/inspector/agents/InspectorHeapAgent.cpp:49 > +InspectorHeapAgent::~InspectorHeapAgent() > +{ > + ErrorString ignored; > + disable(ignored); > +} Ahh, this should be in willDestroyFrontendAndBackend instead of ~InspectorHeapAgent. This is likely what is causing the "InspectorFrontendAPI" not available messages if this agent was accidentally left enabled despite being closed and reopening. Which seems to be the case otherwise the ASSERT(m_enabled) below would have triggered.
Joseph Pecoraro
Comment 19 2015-10-14 18:54:10 PDT
Created attachment 263132 [details] [PATCH] Proposed Fix This fixes the issues above. The actual diffs are pretty small but I forgot to get a diff of just the changes so I'll call them out here. 1. InspectorHeapAgent::didGarbageCollect triggers event asynchronously (RunLoop::current().dispatch(...)) To avoid the MarkedSpace::sweep ASSERTION we dispatch the garbageCollected events asynchronously to avoid executing any JavaScript between the Garbage Collection and Sweeping phases. Sweeping (MarkedSpace.sweep) has never expected to see anything other than "Marked" MarkedBlocks during a "SweepOnly" sweep. Lets not add that possibility/capability to the sweeping and instead avoid it and run JavaScript later. This means that garbageCollected events now appear on the frontend after the script completes execution instead of during. This makes the UI a little more consistent during very long running scripts, but can be seen as a bit of a downside as it would be nice to inform the frontend even while scripts are running (e.g. a near infinite loop being traced). The Heap.gc test needed to be rewritten to allow for gcs between the "HeapAgent.gc()" call and receiving the triggered "Full" Heap.garbageCollected event because this is no longer guaranteed to be synchronous. 2. Disable the InspectorHeapAgent in willDestroyFrontendAndBackend instead of ~InspectorHeapAgent as mentioned before.
Blaze Burg
Comment 20 2015-10-15 10:25:04 PDT
Comment on attachment 263132 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=263132&action=review r=me, as second reviewer. Please address nits and stick around for bots when you land this. > LayoutTests/inspector/heap/garbageCollected.html:38 > + description: "Evaluating allocation heavy JavaScript should trigger Heap.garbageCollected events.", 'allocation-heavy' > LayoutTests/inspector/heap/garbageCollected.html:54 > + InspectorTest.log("PASS: Should see both a Partial and Full Garbage Collection."); How deterministic is this? I can imagine this breaking whenever the GC heuristics change. > LayoutTests/inspector/heap/gc.html:10 > + InspectorTest.dumpActivityToSystemConsole = true; Oops, I think this should be removed, right? > LayoutTests/inspector/heap/gc.html:17 > + HeapAgent.gc(); have you considered HeapAgent.collectGarbage() ? Do we want to expose full vs partial? Which one does this trigger? Best to document it in the protocol file. > Source/JavaScriptCore/heap/HeapObserver.h:-26 > -WebInspector.SourceCodeTimelineTreeElement = class SourceCodeTimelineTreeElement extends WebInspector.TimelineRecordTreeElement This is a very unexpected diff. What happened? > Source/JavaScriptCore/inspector/agents/InspectorHeapAgent.cpp:117 > + double endTime = m_environment.executionStopwatch()->elapsedTime(); Yay. > Source/JavaScriptCore/inspector/agents/InspectorHeapAgent.cpp:127 > + auto collection = Inspector::Protocol::Heap::GarbageCollection::create() Should we be using the weakThis pattern here? What if the inspector tears down before this is dispatched? > Source/JavaScriptCore/inspector/protocol/Heap.json:27 > + "description": "Trigger a full garbage collection." Oh I see, it is documented here. Still, the name is rather terse (if traditional). > Source/WebInspectorUI/ChangeLog:9 > + (Array.prototype.parition): 'partition' > Source/WebInspectorUI/ChangeLog:45 > + The GCs bar will be on top of the the Scripts bar. This is particularly useful 'will be on top of' do you mean overlaid? > Source/WebInspectorUI/UserInterface/Controllers/HeapManager.js:-26 > -WebInspector.SourceCodeTimelineTreeElement = class SourceCodeTimelineTreeElement extends WebInspector.TimelineRecordTreeElement I guess your version control thought this was a copy instead of new file. I blame the copyright blocks. > Source/WebInspectorUI/UserInterface/Models/ScriptTimelineRecord.js:170 > +WebInspector.ScriptTimelineRecord.EventType.displayName = function(eventType, details, includeDetailsInformationInMainTitle) 'includeDetailsInMainTitle' ? (details are information) > Source/WebInspectorUI/UserInterface/Views/ScriptTimelineOverviewGraph.js:77 > + let [gcRecords, nonGCRecords] = this._scriptTimeline.records.partition((x) => x.isGarbageCollection()); beautiful! > Source/WebInspectorUI/UserInterface/Views/ScriptTimelineView.js:228 > + var startTime = this.startTime; 'let' ?
Joseph Pecoraro
Comment 21 2015-10-15 11:12:30 PDT
Comment on attachment 263132 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=263132&action=review >> LayoutTests/inspector/heap/garbageCollected.html:54 >> + InspectorTest.log("PASS: Should see both a Partial and Full Garbage Collection."); > > How deterministic is this? I can imagine this breaking whenever the GC heuristics change. Periodically trigger GCs does a lot of work repeatedly every 20ms. Yes, if GC heuristics change then this could fail. I think it would be good to have a test for this though, as it could actually catch a GC regression if a test like this changes behavior unexpectedly. >> LayoutTests/inspector/heap/gc.html:10 >> + InspectorTest.dumpActivityToSystemConsole = true; > > Oops, I think this should be removed, right? Oops, yes. >> Source/JavaScriptCore/heap/HeapObserver.h:-26 >> -WebInspector.SourceCodeTimelineTreeElement = class SourceCodeTimelineTreeElement extends WebInspector.TimelineRecordTreeElement > > This is a very unexpected diff. What happened? Git thinks a lot of these files were copied from other files... >> Source/JavaScriptCore/inspector/agents/InspectorHeapAgent.cpp:127 >> + auto collection = Inspector::Protocol::Heap::GarbageCollection::create() > > Should we be using the weakThis pattern here? What if the inspector tears down before this is dispatched? I considered this too unlikely. The RunLoop dispatching looks like it would happen (on all ports) at the end of the current run loop before the next run loop, which I thought would be too early for some kind of teardown. That said, it wouldn't hurt to be safe. I didn't know about the weakThis (createWeakPtr) pattern used elsewhere and that certainly looks simple enough to take advantage of here. >> Source/WebInspectorUI/ChangeLog:45 >> + The GCs bar will be on top of the the Scripts bar. This is particularly useful > > 'will be on top of' > > do you mean overlaid? Yes, I'll reword.
Joseph Pecoraro
Comment 22 2015-10-15 11:46:45 PDT
Created attachment 263175 [details] [PATCH] For Landing
WebKit Commit Bot
Comment 23 2015-10-15 11:48:19 PDT
Comment on attachment 263175 [details] [PATCH] For Landing Rejecting attachment 263175 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 263175, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: le without the binary data in line: "Binary files a/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js and b/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js differ". Be sure to use the --binary flag when invoking "git diff" with diffs containing binary files. at /Volumes/Data/EWS/WebKit/Tools/Scripts/VCSUtils.pm line 804, <ARGV> line 1162. Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 25 cwd: /Volumes/Data/EWS/WebKit Full output: http://webkit-queues.webkit.org/results/292205
Joseph Pecoraro
Comment 24 2015-10-15 20:25:39 PDT
WeakPtrFactory doesn't seem to like the multi-threaded nature of JSContext's. <https://build.webkit.org/builders/Apple%20Yosemite%20LLINT%20CLoop%20%28BuildAndTest%29/builds/10041/steps/webkit-jsc-cloop-test/logs/stdio> > ASSERTION FAILED: m_boundThread == currentThread() > /Volumes/Data/slave/yosemite-cloop-debug/build/WebKitBuild/Debug/usr/local/include/wtf/WeakPtr.h(65) : void WTF::WeakReference<Inspector::InspectorHeapAgent>::clear() [T = Inspector::InspectorHeapAgent] > 1 0x107ef1c10 WTFCrash > 2 0x107a861ae WTF::WeakReference<Inspector::InspectorHeapAgent>::clear() > 3 0x107a860f1 WTF::WeakPtrFactory<Inspector::InspectorHeapAgent>::~WeakPtrFactory() > 4 0x107a85515 WTF::WeakPtrFactory<Inspector::InspectorHeapAgent>::~WeakPtrFactory() > 5 0x107a831b4 Inspector::InspectorHeapAgent::~InspectorHeapAgent() ... So I dropped the WeakPtr approach. in a follow-up.
Joseph Pecoraro
Comment 25 2015-10-15 20:27:12 PDT
<http://trac.webkit.org/changeset/191159> - Relanded <http://trac.webkit.org/changeset/191161> - Removed WeakPtr approach (ASSERTs) <http://trac.webkit.org/changeset/191162> - Build Fix after dropping WeakPtr <http://trac.webkit.org/changeset/191164> - EFL Build Fix 1 <http://trac.webkit.org/changeset/191165> - EFL Build Fix 2 Woohoo! First Release bot has succeeded without inspector/heap test issues. I'll wait a bit for Debug bots.
Joseph Pecoraro
Comment 26 2015-10-15 20:55:36 PDT
<http://trac.webkit.org/changeset/191166> - Made inspector/heap/gc.html more reliable
Note You need to log in before you can comment on or make changes to this bug.