Bug 653255: more documentation for JSONClass.cpp (r=fklockii).
authorFelix Klock II <fklockii@adobe.com>
Wed, 05 Sep 2012 05:18:39 -0700
changeset 7539 90ecd685dbfd6a2cee6a08f3dc48184bcc7552e2
parent 7538 73cd547535966365fb984cf4ea5eb0d4c0ddd789
child 7540 bdedfbb2568f143eb1aef8aca7524007e9ee12f4
push id4257
push userdschaffe@adobe.com
push dateThu, 06 Sep 2012 14:36:05 +0000
reviewersfklockii
bugs653255, 1108591
Bug 653255: more documentation for JSONClass.cpp (r=fklockii). {{mercurial 5ef1fd97d28afc89930925f51e50b426bfe086ab}} CL@1108591
core/JSONClass.cpp
--- a/core/JSONClass.cpp
+++ b/core/JSONClass.cpp
@@ -190,16 +190,18 @@ namespace avmplus
         bool m_indexValidForText;       // Implies m_i indexes m_text correctly
         char m_token;                   // The current token
 
         String* m_value;                // Any value associated with
                                         // the current token (for
                                         // numbers and strings)
     };
 
+    // The purpose of ReturnCondition is to signal what end-result
+    // we have ended up in from the stringify call.
     enum ReturnCondition {
         kSomeOutput,      // normal case; emitted > 0 characters
         kNoOutput,        // emitted *zero* characters
         kCycleTraversed,  // cyclic traversal, prepare to throw exception
         kThrewException   // callback threw an exception; percolating up
     };
 
 #define RET_EXN_CHECK(__x__)                                        \
@@ -341,16 +343,35 @@ namespace avmplus
 
         // if value is parent of object in current traversal then
         // returns true.  Otherwise marks value as parent of
         // objects in current traversal and returns false.
         bool valueActive(ScriptObject* value);
         // unmarks value (ie not parent of any object in current traversal)
         void valueNotActive(ScriptObject* value);
 
+        // The purpose of AutoDestructingAtomArray is to build up a
+        // worklist of property names to traverse during stringify.
+        // (A hypothetical direct traversal that recursively processes
+        //  the properties easily hits stack overflow problems.)
+        //
+        // Note that it allocates the worklist via the given FixedMalloc
+        // allocator and automatically frees the worklist during destruction;
+        // thus the stringify kernel must be careful to catch any AS3
+        // exception from a callback and unroll our stack properly
+        // before rethrowing the exception.
+        //
+        // Note also that it is an anti-pattern to be storing Atoms in
+        // the worklist (a FixedMalloc-allocated array) in this
+        // fashion, because the worklist will not be traced by the
+        // garbage collector.  The argument justifying it in this case
+        // is that all of these atoms are property names that are kept
+        // alive by the object we are stringifying, but that is not
+        // a great argument.  Also this code would completely break
+        // in the presence of a copying garbage collector.
         struct AutoDestructingAtomArray {
             AutoDestructingAtomArray(MMgc::FixedMalloc* fm, int32_t atomCount)
                 : m_atoms(NULL)
                 , m_atomCount(atomCount)
                 , m_fixedmalloc(fm)
             {
                 if (atomCount > 0)
                     m_atoms = (Atom*)fm->Alloc(atomCount*sizeof(Atom));
@@ -361,16 +382,29 @@ namespace avmplus
                     m_fixedmalloc->Free(m_atoms);
             }
 
             Atom*              m_atoms;
             int32_t            m_atomCount;
             MMgc::FixedMalloc* m_fixedmalloc;
         };
 
+        // The purpose of Rope is to allow the stringify code to emit
+        // its output incrementally.  Since we do not know a priori
+        // how much output may be associated with a particular input,
+        // we cannot preallocate the output buffer.  So we avoid the
+        // O(n lg n) time blowup (where n is output size) associated
+        // with automatically resizing (and recopying) the target
+        // buffer every time the emission hits the string's capacity
+        // threshold.  Instead we emit to a sequence of fixed length
+        // buffers, and then after emission is complete, we assemble
+        // the final output string in one pass, thus maintaining a
+        // O(n) time bound.  (Unlike some other rope structures, this
+        // uses a list rather than a tree; this works because the
+        // use-case is quite specialized here.)
         struct Rope
         {
             struct Chunk
             {
                 typedef utf8_t bufelem_t;
 
                 // Arbitrarily picking BUF_SIZE such that
                 // sizeof(Chunk) is approximately 4096 bytes.