Bug 1438121: Part 6: Improve comments for js::LiveSavedFrameCache. DONTBUILD r=fitzgen
authorJim Blandy <jimb@mozilla.com>
Sat, 24 Feb 2018 13:57:41 -0800
changeset 763460 8fd9c1479159335e4d4e9d71e2bf99d1d62b8ba7
parent 763459 18cd69ebc9579635e748dbe8c93704b327565b8e
child 763461 7d93207d01801b99e5a4464145f61a02e98b701f
push id101458
push userbmo:rbarker@mozilla.com
push dateMon, 05 Mar 2018 23:59:13 +0000
reviewersfitzgen
bugs1438121
milestone60.0a1
Bug 1438121: Part 6: Improve comments for js::LiveSavedFrameCache. DONTBUILD r=fitzgen MozReview-Commit-ID: C4W5cgwdNtv
js/src/vm/Stack.h
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -1114,23 +1114,20 @@ struct DefaultHasher<AbstractFramePtr> {
 // SavedFrames in the hash table. This stack walking is slow, and we would like
 // to minimize it.
 //
 // We have reserved a bit on most of SpiderMonkey's various frame
 // representations (the exceptions being asm and inlined ion frames). As we
 // create SavedFrame objects for live stack frames in SavedStacks::insertFrames,
 // we set this bit and append the SavedFrame object to the cache. As we walk the
 // stack, if we encounter a frame that has this bit set, that indicates that we
-// have already captured a SavedFrame object for the given stack frame (but not
-// necessarily the current pc) during a previous call to insertFrames. We know
-// that the frame's parent was also captured and has its bit set as well, but
-// additionally we know the parent was captured at its current pc. For the
-// parent, rather than continuing the expensive stack walk, we do a quick and
-// cache-friendly linear search through the frame cache. Upon finishing search
-// through the frame cache, stale entries are removed.
+// have already captured a SavedFrame object for the given stack frame during a
+// previous call to insertFrames. Rather than continuing the expensive stack
+// walk, we do a quick and cache-friendly linear search through the frame cache.
+// Upon finishing the search, stale entries are removed.
 //
 // The frame cache maintains the invariant that its first E[0] .. E[j-1]
 // entries are live and sorted from oldest to younger frames, where 0 < j < n
 // and n = the length of the cache. When searching the cache, we require
 // that we are considering the youngest live frame whose bit is set. Every
 // cache entry E[i] where i >= j is a stale entry. Consider the following
 // scenario:
 //
@@ -1149,20 +1146,32 @@ struct DefaultHasher<AbstractFramePtr> {
 // cache short circuit. Next we proceed to Q and find that it has its bit
 // set, and it is therefore the youngest live frame with its bit set. We
 // search through the frame cache from oldest to youngest and find the cache
 // entry matching Q. We know that T is the next younger live frame from Q
 // and that T does not have an entry in the frame cache because its bit was
 // not set. Therefore, we have found entry E[j-1] and the subsequent entries
 // are stale and should be purged from the frame cache.
 //
+// Note that the youngest frame with a valid entry may have run some code and
+// advanced to a different pc. Each cache entry records the pc for which its
+// SavedFrame is appropriate, and a pc mismatch causes the entry to be purged.
+//
 // We have a LiveSavedFrameCache for each activation to minimize the number of
 // entries that must be scanned through, and to avoid the headaches of
 // maintaining a cache for each compartment and invalidating stale cache entries
 // in the presence of cross-compartment calls.
+//
+// The entire chain of SavedFrames for a given stack capture is created in the
+// compartment of the code that requested the capture, *not* in that of the
+// frames it represents, so in general, different compartments may have
+// different SavedFrame objects representing the same actual stack frame. The
+// LiveSavedFrameCache simply records whichever SavedFrames were created most
+// recently, so if there's a compartment mismatch, we throw away the whole
+// cache.
 class LiveSavedFrameCache
 {
   public:
     using FramePtr = mozilla::Variant<AbstractFramePtr, jit::CommonFrameLayout*>;
 
   private:
     struct Entry
     {