From 4a2ddf8075937b8f5e020dff830a7c01d891c448 Mon Sep 17 00:00:00 2001 From: Milo Yip Date: Sun, 17 Aug 2014 18:29:37 +0800 Subject: [PATCH 1/5] Add Stack::ShrinkToFit() and some refactoring. Lazy allocation. (so Document not for parsing don't need to allocate stack). Remove redundant stackCapacity member variable. --- include/rapidjson/internal/stack.h | 75 ++++++++++++++++++------------ 1 file changed, 46 insertions(+), 29 deletions(-) diff --git a/include/rapidjson/internal/stack.h b/include/rapidjson/internal/stack.h index 4ab84b1..294b51a 100644 --- a/include/rapidjson/internal/stack.h +++ b/include/rapidjson/internal/stack.h @@ -33,67 +33,84 @@ namespace internal { template class Stack { public: - Stack(Allocator* allocator, size_t stack_capacity) : allocator_(allocator), own_allocator_(0), stack_(0), stack_top_(0), stack_end_(0), stack_capacity_(stack_capacity) { - RAPIDJSON_ASSERT(stack_capacity_ > 0); + // Optimization note: Do not allocate memory for stack_ in constructor. + // Do it lazily when first Push() -> Expand() -> Resize(). + Stack(Allocator* allocator, size_t stackCapacity) : allocator_(allocator), ownAllocator(0), stack_(0), stackTop_(0), stackEnd_(0), initialCapacity_(stackCapacity) { + RAPIDJSON_ASSERT(stackCapacity > 0); if (!allocator_) - own_allocator_ = allocator_ = new Allocator(); - stack_top_ = stack_ = (char*)allocator_->Malloc(stack_capacity_); - stack_end_ = stack_ + stack_capacity_; + ownAllocator = allocator_ = new Allocator(); } ~Stack() { Allocator::Free(stack_); - delete own_allocator_; // Only delete if it is owned by the stack + delete ownAllocator; // Only delete if it is owned by the stack } - void Clear() { /*stack_top_ = 0;*/ stack_top_ = stack_; } + void Clear() { stackTop_ = stack_; } + + void ShrinkToFit() { + if (Empty()) { + // If the stack is empty, completely deallocate the memory. + Allocator::Free(stack_); + stack_ = 0; + stackTop_ = 0; + stackEnd_ = 0; + } + else + Resize(GetSize()); + } // Optimization note: try to minimize the size of this function for force inline. // Expansion is run very infrequently, so it is moved to another (probably non-inline) function. template RAPIDJSON_FORCEINLINE T* Push(size_t count = 1) { // Expand the stack if needed - if (stack_top_ + sizeof(T) * count >= stack_end_) + if (stackTop_ + sizeof(T) * count >= stackEnd_) Expand(count); - T* ret = reinterpret_cast(stack_top_); - stack_top_ += sizeof(T) * count; + T* ret = reinterpret_cast(stackTop_); + stackTop_ += sizeof(T) * count; return ret; } template T* Pop(size_t count) { RAPIDJSON_ASSERT(GetSize() >= count * sizeof(T)); - stack_top_ -= count * sizeof(T); - return reinterpret_cast(stack_top_); + stackTop_ -= count * sizeof(T); + return reinterpret_cast(stackTop_); } template T* Top() { RAPIDJSON_ASSERT(GetSize() >= sizeof(T)); - return reinterpret_cast(stack_top_ - sizeof(T)); + return reinterpret_cast(stackTop_ - sizeof(T)); } template T* Bottom() { return (T*)stack_; } Allocator& GetAllocator() { return *allocator_; } - bool Empty() const { return stack_top_ == stack_; } - size_t GetSize() const { return static_cast(stack_top_ - stack_); } - size_t GetCapacity() const { return stack_capacity_; } + bool Empty() const { return stackTop_ == stack_; } + size_t GetSize() const { return static_cast(stackTop_ - stack_); } + size_t GetCapacity() const { return static_cast(stackEnd_ - stack_); } private: template void Expand(size_t count) { - size_t new_capacity = stack_capacity_ * 2; - size_t size = GetSize(); - size_t new_size = GetSize() + sizeof(T) * count; - if (new_capacity < new_size) - new_capacity = new_size; - stack_ = (char*)allocator_->Realloc(stack_, stack_capacity_, new_capacity); - stack_capacity_ = new_capacity; - stack_top_ = stack_ + size; - stack_end_ = stack_ + stack_capacity_; + // Only expand the capacity if the current stack exists. Otherwise just create a stack with initial capacity. + size_t newCapacity = (stack_ == 0) ? initialCapacity_ : GetCapacity() * 2; + size_t newSize = GetSize() + sizeof(T) * count; + if (newCapacity < newSize) + newCapacity = newSize; + + Resize(newCapacity); + } + + void Resize(size_t newCapacity) { + const size_t size = GetSize(); // Backup the current size + stack_ = (char*)allocator_->Realloc(stack_, GetCapacity(), newCapacity); + stackTop_ = stack_ + size; + stackEnd_ = stack_ + newCapacity; } // Prohibit copy constructor & assignment operator. @@ -101,11 +118,11 @@ private: Stack& operator=(const Stack&); Allocator* allocator_; - Allocator* own_allocator_; + Allocator* ownAllocator; char *stack_; - char *stack_top_; - char *stack_end_; - size_t stack_capacity_; + char *stackTop_; + char *stackEnd_; + size_t initialCapacity_; }; } // namespace internal From 2e23787753c425569ddca967ba8e1c35ef23eb5a Mon Sep 17 00:00:00 2001 From: Milo Yip Date: Sun, 17 Aug 2014 18:31:41 +0800 Subject: [PATCH 2/5] Change Reader/Writer's stack allocator to CrtAllocator --- include/rapidjson/reader.h | 12 ++++++------ include/rapidjson/writer.h | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/include/rapidjson/reader.h b/include/rapidjson/reader.h index 584ad65..597e6f4 100644 --- a/include/rapidjson/reader.h +++ b/include/rapidjson/reader.h @@ -347,9 +347,9 @@ template<> inline void SkipWhitespace(StringStream& is) { \tparam SourceEncoding Encoding of the input stream. \tparam TargetEncoding Encoding of the parse output. - \tparam Allocator Allocator type for stack. + \tparam StackAllocator Allocator type for stack. */ -template > +template class GenericReader { public: typedef typename SourceEncoding::Ch Ch; //!< SourceEncoding character type @@ -358,7 +358,7 @@ public: /*! \param allocator Optional allocator for allocating stack memory. (Only use for non-destructive parsing) \param stackCapacity stack capacity in bytes for storing a single decoded string. (Only use for non-destructive parsing) */ - GenericReader(Allocator* allocator = 0, size_t stackCapacity = kDefaultStackCapacity) : stack_(allocator, stackCapacity), parseResult_() {} + GenericReader(StackAllocator* stackAllocator = 0, size_t stackCapacity = kDefaultStackCapacity) : stack_(stackAllocator, stackCapacity), parseResult_() {} //! Parse JSON text. /*! \tparam parseFlags Combination of \ref ParseFlag. @@ -591,12 +591,12 @@ private: public: typedef typename TargetEncoding::Ch Ch; - StackStream(internal::Stack& stack) : stack_(stack), length_(0) {} + StackStream(internal::Stack& stack) : stack_(stack), length_(0) {} RAPIDJSON_FORCEINLINE void Put(Ch c) { *stack_.template Push() = c; ++length_; } - internal::Stack& stack_; + internal::Stack& stack_; SizeType length_; private: @@ -1317,7 +1317,7 @@ private: } static const size_t kDefaultStackCapacity = 256; //!< Default stack capacity in bytes for storing a single decoded string. - internal::Stack stack_; //!< A stack for storing decoded string temporarily during non-destructive parsing. + internal::Stack stack_; //!< A stack for storing decoded string temporarily during non-destructive parsing. ParseResult parseResult_; }; // class GenericReader diff --git a/include/rapidjson/writer.h b/include/rapidjson/writer.h index 7927a0c..c844853 100644 --- a/include/rapidjson/writer.h +++ b/include/rapidjson/writer.h @@ -49,10 +49,10 @@ namespace rapidjson { \tparam OutputStream Type of output stream. \tparam SourceEncoding Encoding of source string. \tparam TargetEncoding Encoding of output stream. - \tparam Allocator Type of allocator for allocating memory of stack. + \tparam StackAllocator Type of allocator for allocating memory of stack. \note implements Handler concept */ -template, typename TargetEncoding = UTF8<>, typename Allocator = MemoryPoolAllocator<> > +template, typename TargetEncoding = UTF8<>, typename StackAllocator = CrtAllocator> class Writer { public: typedef typename SourceEncoding::Ch Ch; @@ -62,10 +62,10 @@ public: \param allocator User supplied allocator. If it is null, it will create a private one. \param levelDepth Initial capacity of stack. */ - Writer(OutputStream& os, Allocator* allocator = 0, size_t levelDepth = kDefaultLevelDepth) : - os_(&os), level_stack_(allocator, levelDepth * sizeof(Level)), hasRoot_(false) {} + Writer(OutputStream& os, StackAllocator* stackAllocator = 0, size_t levelDepth = kDefaultLevelDepth) : + os_(&os), level_stack_(stackAllocator, levelDepth * sizeof(Level)), hasRoot_(false) {} - Writer(Allocator* allocator = 0, size_t levelDepth = kDefaultLevelDepth) : + Writer(StackAllocator* allocator = 0, size_t levelDepth = kDefaultLevelDepth) : os_(0), level_stack_(allocator, levelDepth * sizeof(Level)), hasRoot_(false) {} //! Reset the writer with a new stream. @@ -327,7 +327,7 @@ protected: } OutputStream* os_; - internal::Stack level_stack_; + internal::Stack level_stack_; bool hasRoot_; private: From 37140198193ab203e716db1b43ce3a697df206dd Mon Sep 17 00:00:00 2001 From: Milo Yip Date: Sun, 17 Aug 2014 18:32:08 +0800 Subject: [PATCH 3/5] Add ShrinkToFit() to StringBuffer and MemoryBuffer --- include/rapidjson/memorybuffer.h | 1 + include/rapidjson/stringbuffer.h | 6 ++++++ test/unittest/writertest.cpp | 1 + 3 files changed, 8 insertions(+) diff --git a/include/rapidjson/memorybuffer.h b/include/rapidjson/memorybuffer.h index 4e82036..b94d2df 100644 --- a/include/rapidjson/memorybuffer.h +++ b/include/rapidjson/memorybuffer.h @@ -49,6 +49,7 @@ struct GenericMemoryBuffer { void Flush() {} void Clear() { stack_.Clear(); } + void ShrinkToFit() { stack_.ShrinkToFit(); } Ch* Push(size_t count) { return stack_.template Push(count); } void Pop(size_t count) { stack_.template Pop(count); } diff --git a/include/rapidjson/stringbuffer.h b/include/rapidjson/stringbuffer.h index b8d7968..bccc247 100644 --- a/include/rapidjson/stringbuffer.h +++ b/include/rapidjson/stringbuffer.h @@ -42,6 +42,12 @@ struct GenericStringBuffer { void Flush() {} void Clear() { stack_.Clear(); } + void ShrinkToFit() { + // Push and pop a null terminator. This is safe. + *stack_.template Push() = '\0'; + stack_.ShrinkToFit(); + stack_.template Pop(1); + } Ch* Push(size_t count) { return stack_.template Push(count); } void Pop(size_t count) { stack_.template Pop(count); } diff --git a/test/unittest/writertest.cpp b/test/unittest/writertest.cpp index e313c8b..c3e9d4e 100644 --- a/test/unittest/writertest.cpp +++ b/test/unittest/writertest.cpp @@ -31,6 +31,7 @@ TEST(Writer, Compact) { StringStream s("{ \"hello\" : \"world\", \"t\" : true , \"f\" : false, \"n\": null, \"i\":123, \"pi\": 3.1416, \"a\":[1, 2, 3] } "); StringBuffer buffer; Writer writer(buffer); + buffer.ShrinkToFit(); Reader reader; reader.Parse<0>(s, writer); EXPECT_STREQ("{\"hello\":\"world\",\"t\":true,\"f\":false,\"n\":null,\"i\":123,\"pi\":3.1416,\"a\":[1,2,3]}", buffer.GetString()); From 941aa93f458139c90ed21e708b59c7a09bc42cbc Mon Sep 17 00:00:00 2001 From: Milo Yip Date: Sun, 17 Aug 2014 18:33:47 +0800 Subject: [PATCH 4/5] Separate Document's value and stack allocator. Use CrtAllocator for stack. ShrinkToFit stack after parsing. --- include/rapidjson/document.h | 34 ++++++++++++++++++++++++---------- test/unittest/documenttest.cpp | 28 +++++++++++++++++++--------- 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/include/rapidjson/document.h b/include/rapidjson/document.h index 880b040..a44c0ea 100644 --- a/include/rapidjson/document.h +++ b/include/rapidjson/document.h @@ -1259,7 +1259,7 @@ int z = a[0u].GetInt(); // This works too. } private: - template + template friend class GenericDocument; enum { @@ -1406,11 +1406,12 @@ typedef GenericValue > Value; //! A document for parsing JSON text as DOM. /*! \note implements Handler concept - \tparam Encoding encoding for both parsing and string storage. - \tparam Allocator allocator for allocating memory for the DOM, and the stack during parsing. - \warning Although GenericDocument inherits from GenericValue, the API does \b not provide any virtual functions, especially no virtual destructors. To avoid memory leaks, do not \c delete a GenericDocument object via a pointer to a GenericValue. + \tparam Encoding Encoding for both parsing and string storage. + \tparam Allocator Allocator for allocating memory for the DOM + \tparam StackAllocator Allocator for allocating memory for stack during parsing. + \warning Although GenericDocument inherits from GenericValue, the API does \b not provide any virtual functions, especially no virtual destructor. To avoid memory leaks, do not \c delete a GenericDocument object via a pointer to a GenericValue. */ -template > +template , typename StackAllocator = CrtAllocator> class GenericDocument : public GenericValue { public: typedef typename Encoding::Ch Ch; //!< Character type derived from Encoding. @@ -1418,10 +1419,20 @@ public: typedef Allocator AllocatorType; //!< Allocator type from template parameter. //! Constructor - /*! \param allocator Optional allocator for allocating stack memory. - \param stackCapacity Initial capacity of stack in bytes. + /*! \param allocator Optional allocator for allocating memory. + \param stackCapacity Optional initial capacity of stack in bytes. + \param stackAllocator Optional allocator for allocating memory for stack. */ - GenericDocument(Allocator* allocator = 0, size_t stackCapacity = kDefaultStackCapacity) : stack_(allocator, stackCapacity), parseResult_() {} + GenericDocument(Allocator* allocator = 0, size_t stackCapacity = kDefaultStackCapacity, StackAllocator* stackAllocator = 0) : + allocator_(allocator), ownAllocator_(0), stack_(stackAllocator, stackCapacity), parseResult_() + { + if (!allocator_) + ownAllocator_ = allocator_ = new Allocator(); + } + + ~GenericDocument() { + delete ownAllocator_; + } //!@name Parse from stream //!@{ @@ -1549,7 +1560,7 @@ public: //!@} //! Get the allocator of this document. - Allocator& GetAllocator() { return stack_.GetAllocator(); } + Allocator& GetAllocator() { return *allocator_; } //! Get the capacity of stack in bytes. size_t GetStackCapacity() const { return stack_.GetCapacity(); } @@ -1612,10 +1623,13 @@ private: (stack_.template Pop(1))->~ValueType(); else stack_.Clear(); + stack_.ShrinkToFit(); } static const size_t kDefaultStackCapacity = 1024; - internal::Stack stack_; + Allocator* allocator_; + Allocator* ownAllocator_; + internal::Stack stack_; ParseResult parseResult_; }; diff --git a/test/unittest/documenttest.cpp b/test/unittest/documenttest.cpp index 0606ccf..2fb5cf2 100644 --- a/test/unittest/documenttest.cpp +++ b/test/unittest/documenttest.cpp @@ -28,48 +28,58 @@ using namespace rapidjson; -TEST(Document, Parse) { - Document doc; +template +void ParseTest() { + typedef GenericDocument, Allocator, StackAllocator> DocumentType; + typedef DocumentType::ValueType ValueType; + DocumentType doc; doc.Parse(" { \"hello\" : \"world\", \"t\" : true , \"f\" : false, \"n\": null, \"i\":123, \"pi\": 3.1416, \"a\":[1, 2, 3, 4] } "); EXPECT_TRUE(doc.IsObject()); EXPECT_TRUE(doc.HasMember("hello")); - Value& hello = doc["hello"]; + const ValueType& hello = doc["hello"]; EXPECT_TRUE(hello.IsString()); EXPECT_STREQ("world", hello.GetString()); EXPECT_TRUE(doc.HasMember("t")); - Value& t = doc["t"]; + const ValueType& t = doc["t"]; EXPECT_TRUE(t.IsTrue()); EXPECT_TRUE(doc.HasMember("f")); - Value& f = doc["f"]; + const ValueType& f = doc["f"]; EXPECT_TRUE(f.IsFalse()); EXPECT_TRUE(doc.HasMember("n")); - Value& n = doc["n"]; + const ValueType& n = doc["n"]; EXPECT_TRUE(n.IsNull()); EXPECT_TRUE(doc.HasMember("i")); - Value& i = doc["i"]; + const ValueType& i = doc["i"]; EXPECT_TRUE(i.IsNumber()); EXPECT_EQ(123, i.GetInt()); EXPECT_TRUE(doc.HasMember("pi")); - Value& pi = doc["pi"]; + const ValueType& pi = doc["pi"]; EXPECT_TRUE(pi.IsNumber()); EXPECT_EQ(3.1416, pi.GetDouble()); EXPECT_TRUE(doc.HasMember("a")); - Value& a = doc["a"]; + const ValueType& a = doc["a"]; EXPECT_TRUE(a.IsArray()); EXPECT_EQ(4u, a.Size()); for (SizeType i = 0; i < 4; i++) EXPECT_EQ(i + 1, a[i].GetUint()); } +TEST(Document, Parse) { + ParseTest, CrtAllocator>(); + ParseTest, MemoryPoolAllocator<> >(); + ParseTest >(); + ParseTest(); +} + static FILE* OpenEncodedFile(const char* filename) { char buffer[1024]; sprintf(buffer, "encodings/%s", filename); From d6513e251c362fa857021f6dd2bfc8605640d220 Mon Sep 17 00:00:00 2001 From: Milo Yip Date: Sun, 17 Aug 2014 18:55:36 +0800 Subject: [PATCH 5/5] Fix a gcc compilation error --- test/unittest/documenttest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unittest/documenttest.cpp b/test/unittest/documenttest.cpp index 2fb5cf2..5809361 100644 --- a/test/unittest/documenttest.cpp +++ b/test/unittest/documenttest.cpp @@ -31,7 +31,7 @@ using namespace rapidjson; template void ParseTest() { typedef GenericDocument, Allocator, StackAllocator> DocumentType; - typedef DocumentType::ValueType ValueType; + typedef typename DocumentType::ValueType ValueType; DocumentType doc; doc.Parse(" { \"hello\" : \"world\", \"t\" : true , \"f\" : false, \"n\": null, \"i\":123, \"pi\": 3.1416, \"a\":[1, 2, 3, 4] } ");