viggox / rapidjson

Automatically exported from code.google.com/p/rapidjson
MIT License
0 stars 0 forks source link

Strange stream object copying while parsing? #78

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
In a rapidjson reader class there's some strange code:

template<typename Stream>
void SkipWhitespace(Stream& stream)
{
    Stream s = stream;  // Use a local copy for optimization
    while (s.Peek() == ' ' || s.Peek() == '\n' || s.Peek() == '\r' || s.Peek() == '\t')
        s.Take();
    stream = s;
}

Why stream object is reassigned two times, what optimization this gives 
comparing to direct accessing it via reference? What if stream object copying 
is a heavy process by itself and also can cause an exception? What if I want to 
create stream class with non-copyable semantic? For me all of this is a clear 
pessimization instead of optimization.

Original issue reported on code.google.com by Anton.Br...@gmail.com on 14 Jun 2013 at 8:52

GoogleCodeExporter commented 8 years ago
It is actually optimized for in-memory string stream. 
I think it should be take it out as specialization for that stream class.
Thank you for your comment.

Original comment by milo...@gmail.com on 18 Jun 2013 at 1:38

GoogleCodeExporter commented 8 years ago
Excuse me, but I still don't understand where does the optimization for string 
stream case come from?
As well usually I expect any stream object to be non-copyable because 
internally it should have some reference to some resource, and this reference 
is not copyable, having some mechanism to share the resource also looks like a 
big design restriction and could be error-prone.

Original comment by serg...@gmail.com on 9 Apr 2014 at 7:35

GoogleCodeExporter commented 8 years ago
Even in

template<unsigned parseFlags, typename InputStream, typename Handler>
void ParseNumber(InputStream& s, Handler& handler) {

The usage of `is` and `s` is mixed.
Will it hurt somehow if I remove any

InputStream s = is; // Use a local copy for optimization
is = s; // Restore is

and change the name of argument `is`->`s` ?

Original comment by serg...@gmail.com on 9 Apr 2014 at 8:14

GoogleCodeExporter commented 8 years ago
It cannot be understood by C++ code itself. The "optimization" for string 
stream is related to how compilers generated the machine code. It is more a 
like a hack.

Original comment by milo...@gmail.com on 9 Apr 2014 at 9:50

GoogleCodeExporter commented 8 years ago
https://github.com/miloyip/rapidjson/issues/30

Original comment by milo...@gmail.com on 29 Jun 2014 at 11:24

GoogleCodeExporter commented 8 years ago

Original comment by milo...@gmail.com on 30 Jun 2014 at 5:18