author | Nick Fitzgerald <fitzgen@gmail.com> |
Tue, 27 Oct 2015 14:13:39 -0700 | |
changeset 269821 | f73d722759c120dc3b53305732cfc0eb5fbcfd56 |
parent 269820 | 7894e641adfa9f527b58b91ad2d194bd73ab18e5 |
child 269822 | e90655d486cac165a2654d66442e2db1c08b538c |
push id | 29592 |
push user | cbook@mozilla.com |
push date | Wed, 28 Oct 2015 09:38:46 +0000 |
treeherder | mozilla-central@872927368b0e [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | froydnj |
bugs | 1218597 |
milestone | 44.0a1 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/devtools/shared/heapsnapshot/CoreDump.proto +++ b/devtools/shared/heapsnapshot/CoreDump.proto @@ -105,17 +105,17 @@ message StackFrame { message Node { optional uint64 id = 1; // De-duplicated two-byte string. oneof TypeNameOrRef { bytes typeName = 2; uint64 typeNameRef = 3; } - + optional uint64 size = 4; repeated Edge edges = 5; optional StackFrame allocationStack = 6; // De-duplicated one-byte string. oneof JSObjectClassNameOrRef { bytes jsObjectClassName = 7; uint64 jsObjectClassNameRef = 8;
--- a/devtools/shared/heapsnapshot/HeapSnapshot.cpp +++ b/devtools/shared/heapsnapshot/HeapSnapshot.cpp @@ -99,28 +99,38 @@ HeapSnapshot::Create(JSContext* cx, template<typename MessageType> static bool parseMessage(ZeroCopyInputStream& stream, MessageType& message) { // We need to create a new `CodedInputStream` for each message so that the // 64MB limit is applied per-message rather than to the whole stream. CodedInputStream codedStream(&stream); + // The protobuf message nesting that core dumps exhibit is dominated by + // allocation stacks' frames. In the most deeply nested case, each frame has + // two messages: a StackFrame message and a StackFrame::Data message. These + // frames are on top of a small constant of other messages. There are a + // MAX_STACK_DEPTH number of frames, so we multiply this by 3 to make room for + // the two messages per frame plus some head room for the constant number of + // non-dominating messages. + codedStream.SetRecursionLimit(HeapSnapshot::MAX_STACK_DEPTH * 3); + // Because protobuf messages aren't self-delimiting, we serialize each message // preceeded by its size in bytes. When deserializing, we read this size and // then limit reading from the stream to the given byte size. If we didn't, // then the first message would consume the entire stream. uint32_t size = 0; if (NS_WARN_IF(!codedStream.ReadVarint32(&size))) return false; auto limit = codedStream.PushLimit(size); if (NS_WARN_IF(!message.ParseFromCodedStream(&codedStream)) || - NS_WARN_IF(!codedStream.ConsumedEntireMessage())) + NS_WARN_IF(!codedStream.ConsumedEntireMessage()) || + NS_WARN_IF(codedStream.BytesUntilLimit() != 0)) { return false; } codedStream.PopLimit(limit); return true; } @@ -904,17 +914,18 @@ class MOZ_STACK_CLASS StreamWriter : pub uint64_t ref = oneByteStringsAlreadySerialized.count(); if (!oneByteStringsAlreadySerialized.add(ptr, string, ref)) return false; setString(stringData.release()); return true; } - protobuf::StackFrame* getProtobufStackFrame(JS::ubi::StackFrame& frame) { + protobuf::StackFrame* getProtobufStackFrame(JS::ubi::StackFrame& frame, + size_t depth = 1) { // NB: de-duplicated string properties must be written in the same order // here as they are read in `HeapSnapshot::saveStackFrame` or else indices // in references to already serialized strings will be off. MOZ_ASSERT(frame, "null frames should be represented as the lack of a serialized " "stack frame"); @@ -952,18 +963,18 @@ class MOZ_STACK_CLASS StreamWriter : pub [&] (std::string* name) { data->set_allocated_functiondisplayname(name); }, [&] (uint64_t ref) { data->set_functiondisplaynameref(ref); })) { return nullptr; } } auto parent = frame.parent(); - if (parent) { - auto protobufParent = getProtobufStackFrame(parent); + if (parent && depth < HeapSnapshot::MAX_STACK_DEPTH) { + auto protobufParent = getProtobufStackFrame(parent, depth + 1); if (!protobufParent) return nullptr; data->set_allocated_parent(protobufParent); } protobufStackFrame->set_allocated_data(data.release()); if (!framesAlreadySerialized.put(id))
--- a/devtools/shared/heapsnapshot/HeapSnapshot.h +++ b/devtools/shared/heapsnapshot/HeapSnapshot.h @@ -66,16 +66,23 @@ class HeapSnapshot final : public nsISup bool saveNode(const protobuf::Node& node); // Save the given `protobuf::StackFrame` message in this `HeapSnapshot` as a // `DeserializedStackFrame`. The saved stack frame's id is returned via the // out parameter. bool saveStackFrame(const protobuf::StackFrame& frame, StackFrameId& outFrameId); +public: + // The maximum number of stack frames that we will serialize into a core + // dump. This helps prevent over-recursion in the protobuf library when + // deserializing stacks. + static const size_t MAX_STACK_DEPTH = 60; + +private: // If present, a timestamp in the same units that `PR_Now` gives. Maybe<uint64_t> timestamp; // The id of the root node for this deserialized heap graph. NodeId rootId; // The set of nodes in this deserialized heap graph, keyed by id. using NodeSet = js::HashSet<DeserializedNode, DeserializedNode::HashPolicy>;
new file mode 100644 --- /dev/null +++ b/devtools/shared/heapsnapshot/tests/unit/test_HeapSnapshot_deepStack_01.js @@ -0,0 +1,70 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +// Test that we can save a core dump with very deep allocation stacks and read +// it back into a HeapSnapshot. + +function stackDepth(stack) { + return stack ? 1 + stackDepth(stack.parent) : 0; +} + +function run_test() { + // Create a Debugger observing a debuggee's allocations. + const debuggee = new Cu.Sandbox(null); + const dbg = new Debugger(debuggee); + dbg.memory.trackingAllocationSites = true; + + // Allocate some objects in the debuggee that will have their allocation + // stacks recorded by the Debugger. + + debuggee.eval("this.objects = []"); + debuggee.eval( + (function recursiveAllocate(n) { + if (n <= 0) + return; + + // Make sure to recurse before pushing the object so that when TCO is + // implemented sometime in the future, it doesn't invalidate this test. + recursiveAllocate(n - 1); + this.objects.push({}); + }).toString() + ); + debuggee.eval("recursiveAllocate = recursiveAllocate.bind(this);"); + debuggee.eval("recursiveAllocate(200);"); + + // Now save a snapshot that will include the allocation stacks and read it + // back again. + + const filePath = ChromeUtils.saveHeapSnapshot({ runtime: true }); + ok(true, "Should be able to save a snapshot."); + + const snapshot = ChromeUtils.readHeapSnapshot(filePath); + ok(snapshot, "Should be able to read a heap snapshot"); + ok(snapshot instanceof HeapSnapshot, "Should be an instanceof HeapSnapshot"); + + const report = snapshot.takeCensus({ + breakdown: { by: "allocationStack", + then: { by: "count", bytes: true, count: true }, + noStack: { by: "count", bytes: true, count: true } + } + }); + + // Keep this synchronized with `HeapSnapshot::MAX_STACK_DEPTH`! + const MAX_STACK_DEPTH = 60; + + let foundStacks = false; + report.forEach((v, k) => { + if (k === "noStack") { + return; + } + + foundStacks = true; + const depth = stackDepth(k); + dumpn("Stack depth is " + depth); + ok(depth <= MAX_STACK_DEPTH, + "Every stack should have depth less than or equal to the maximum stack depth"); + }); + ok(foundStacks); + + do_test_finished(); +}
--- a/devtools/shared/heapsnapshot/tests/unit/xpcshell.ini +++ b/devtools/shared/heapsnapshot/tests/unit/xpcshell.ini @@ -29,16 +29,17 @@ support-files = [test_HeapAnalyses_takeCensus_01.js] [test_HeapAnalyses_takeCensus_02.js] [test_HeapAnalyses_takeCensus_03.js] [test_HeapAnalyses_takeCensus_04.js] [test_HeapAnalyses_takeCensus_05.js] [test_HeapAnalyses_takeCensus_06.js] [test_HeapAnalyses_takeCensus_07.js] [test_HeapSnapshot_creationTime_01.js] +[test_HeapSnapshot_deepStack_01.js] [test_HeapSnapshot_takeCensus_01.js] [test_HeapSnapshot_takeCensus_02.js] [test_HeapSnapshot_takeCensus_03.js] [test_HeapSnapshot_takeCensus_04.js] [test_HeapSnapshot_takeCensus_05.js] [test_HeapSnapshot_takeCensus_06.js] [test_HeapSnapshot_takeCensus_07.js] [test_HeapSnapshot_takeCensus_08.js]