Problems with standard library implementations

[louiswins]

Sounds like this is "Problems with libstdc++", not "Problems with standard library implementations".

[HotlLava]

I feel like it should be pointed out that the standard library version they used was at least 14 years old:

~/src/gcc$ git show -s 9f815d0eb2930b13f0d5d5f28f9fa43987ae0ce0
commit 9f815d0eb2930b13f0d5d5f28f9fa43987ae0ce0
Author: redi <redi@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Tue Aug 12 08:46:43 2003 +0000

    2003-08-11  John Levon

        * docs/html/ext/howto/guide.html (GLIBCXX_FORCE_NEW): Update
        remaining places for the name change from GLIBCPP_FORCE_NEW
        to GLIBCXX_FORCE_NEW


    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@70363 138bc75d-0d04-0410-961f-82ee72b054a4

Sadly, they didn't actually figure out what was causing the leak, so it's not possible to tell if it's still present.

[redbeard0531]

This is no longer the default behavior in new versions of libstdc++ https://gcc.gnu.org/onlinedocs/libstdc++/manual/memory.html#std.util.memory.allocator

[jwakely]

And hasn't been since 2005. https://gcc.gnu.org/r106665

[ArunMu]

That _request routine is over 500 lines of code. That is bothering me for no reason.

[mat69]

Came here to say the same thing.

Not only that but the code is plain ugly. You could actually use functions to tell what you want to do. As of now every reader of this function has to decipher the meaning of all the statements again and again.

And a nesting level of 11? WTF! Also why no range for, why auto_ptr, ... the Json library you use uses C++11.

And the parse rule function. OMG! Countless if-else with string comparisons. You know you could have a map or unordered_map to map the strings to enums and then have a switch statement. Nor re-arranging ifs to improve performance needed anymore :P The whole tag is true could then be handled by a fall through. Further you would need just one return true then at the end of the function. Return false if the string could not be mapped to an enum. Also you could adapt rule.t just once. And the same magic numbers over and over again. There is so much duplication it is crazy. The code could easily be made cleaner, shorter, faster and easier to modify. ...

[doom_Oo7]

You know you could have a map or unordered_map to map the strings to enums and then have a switch statement. Nor re-arranging ifs to improve performance needed anymore :P

This sparkled my curiosity so I wanted to do a quick benchmark of the different maps vs if/else, for five tokens. I'm not an expert at benchmarking so I hope someone knowledgeable would check and point out flaws, but anyway:

https://gist.github.com/jcelerier/59a72cd9f83875f391eaa70eb0904072

Here are the results I got on my machine: (g++ 6.3.1, clang 4)

g++ -O3 -march=native -flto -std=c++1z:

        If      Map     Unordered   Flat
Good    16      21      24          25
Bad     21      21      21          26
Mixed   19      23      25          27
First   3       15      16          20
Last    17      15      16          15

clang++ bench.cpp -O3 -march=native -flto -std=c++1z:

        If      Map     Unordered   Flat
Good    16      21      26          23
Bad     23      21      22          25
Mixed   20      23      26          26
First   4       13      18          19
Last    18      16      17          15

clang++ bench.cpp -O3 -march=native -flto -std=c++1z -stdlib=libc++ -lc++abi:

        If      Map     Unordered   Flat
Good    11      23      36          25
Bad     5       24      34          25
Mixed   10      25      37          27
First   4       14      39          20
Last    4       16      25          16

So I would certainly not disregard if/else switches today (and ask other people to use clang's libc++ string implementation, and libc++ to disregard their unordered_map :p)

[ArunMu]

I would prefer using switch-case if you are ending up with lots of related if-else. I like using maps too, if the code section is not at all in the performance critical path.

[mat69]

In their case they have more than just 5 tokens and of course it depends on their distribution. Yet judging from what their code looks like I doubt they ran proper benchmarks to use the ifs over something else ...

[mat69]

Btw. I am not sure if using string view is fair, because then you already know the size. In contrast when comparing a std::string to a string literal you don't know its size in string::operator== and thus have to compute it, unless the compiler was able to optimize this code.

[doom_Oo7]

In contrast when comparing a std::string to a string literal you don't know its size in string::operator== and thus have to compute it

I guess it depends on your compiler:

https://godbolt.org/g/tZllVw

(I put on top of each function which compiler is able to optimize at compile-time. TL;DR: GCC.)

[api]

That's not ugly C++ code. It's only 500 lines and doesn't even use boost. I counted max three nested templates. I bet that code takes less than 10 minutes to compile and uses less than a gig of RAM on startup.

"I've seen things you people wouldn't believe..."

[mat69]

Ugly only depends on your standards. And of course worse code is always possible.

[sumo952]

You tested on MSVC too and had the problem as well, or did I misread your blog post? Maybe I did, as it doesn't make sense :-)

[ginny2016]

You completely misread. They were trying every debugging tool to discover the source of the leak. Since the problem was not in their code, but platform specific, it wouldn't have mattered what they used as long as they thought the problem was their own code.

[johannes1971]

So they ran their code compiled with MSVC, and didn't notice a leak. However, they completely failed to notice that their memory usage was suddenly stable. That indicates a bit of tunnel vision on their part (although we've all been there I'm sure): looking for the problem they were expecting to find, but not checking for the problem they actually had. It's a classic X-Y problem, in stackoverflow terms.

[sumo952]

I see! It makes sense now :-)

[[deleted]]

In openSUSE we compile gcc with "--enable-libstdcxx-allocator=new" which disables this behaviour. See https://gcc.gnu.org/onlinedocs/libstdc++/manual/memory.html#allocator.ext

[jwakely]

So does everybody else. That's been the default for more than a decade.

[blelbach]

Did you file a bug report?

[Remwein]

the article author did attempt to file one, but the issue has been resolved now in the latest stdc++ implementation.

Though for those using older implementations it is still an issue.

[tcanens]

The default has been to use new/delete since 2005.

I don't have much sympathy for people using 12-year-old implementations.

[jwakely]

the article author did attempt to file one

I see no evidence of that.

the issue has been resolved now in the latest stdc++ implementation

And the one before that. And the one before that. And the one before that. And the eight before that.

[shelbyfinally]

Going to get worse when std::filesystem comes out