Tencent / rapidjson

A fast JSON parser/generator for C++ with both SAX/DOM style API
http://rapidjson.org/
Other
14.22k stars 3.53k forks source link

why `copy` always true in String()/Key() of SAX mode parsing? #737

Open hh9527 opened 8 years ago

hh9527 commented 8 years ago

Is there any reason to copy the content of string even if there is no escape at all?

miloyip commented 8 years ago

No, copy is not always true. It does not depend on escaping, it is only depends on whether kParseInsituFlag is enabled. When kParseInsituFlag is enabled, copy = false. When kParseInsituFlag is disabled, copy = true. If we are not using insitu parsing, we still need to append \0 at the end of string to replace the closing ". This is mainly for making it compatible as a null-terminated string.

hh9527 commented 8 years ago

@miloyip 叶老师,您好

So, there is no possibility to avoid any memory management for now? I think it would be very useful if there is a mode to support zero memory management and zero copy.

Something like this:


struct MyParseHandler {
  // only be called when there is no escape chars
  bool String(const char* s, SizeType len) { ... }

  // only be called when there are some escaped chars
  bool EscapedStringSlice(const char* s, SizeType len) { ... }
  bool EscapedChar(uint32_t ch) { ... }
  bool EscapedStringEnd() { ... }

  // other callbacks ...
};

And when parse "abc\t\u1234xyz\r\n", the following callbacks are emitted

EscapedStringSlice("abc", 3);
EscapedChar('\t');
EscapedChar(1234);
EscapedStringSlice("xyz", 3);
EscapedChar('\r');
EscapedChar('\n');
EscapedStringEnd();

And C-style null-terminated string is not necessary in every case. It would be better to make it configurable via template parameter or kFlags.

Of course, It is only my point of view, I am not sure whether it will impact the performance.

firewave commented 5 years ago

I just looked into using kParseInsituFlag and although it gave small boost I am still seeing the Writer::String() method in the profiler. It turns out, that Writer::String is ignoring the copy parameter - this is true for the 1.1.0 release and the current master.