menoh icon indicating copy to clipboard operation
menoh copied to clipboard

Consider switching to c++11

Open enobufs opened this issue 6 years ago • 3 comments

I noticed that current AWS Lambda did not support libstdc++.so with GLIBCXX_3.4.20 or greater, which means all dependencies must be compiled with gcc < v5.0. (C++14 is not supported) It is a great disadvantage for Menoh since AWS Lambda is very popular now...

One of the obvious workarounds would be to put new version of libstdc++.so and all of its dependencies inside the Lambda deployment package, however, that increases the size of the package unnecessarily, and it makes creating such package a lot more difficult for developers.

I believe, future AWS Lambda would allow gcc >= v5.0 in the future, but such plan is unknown. A similar problem could occur in other use cases than AWS Lambda also. In order to increase usability/applicability of Menoh, we may need to consider making Menoh-core C++11 compatible.

Note: Other dependencies, such as mkl-dnn, protobuf, can be compiled with gcc < v5.0 with no problem.

enobufs avatar Jul 26 '18 03:07 enobufs

Hi. I made a branch that is the modified master to be compiled with C++11. You can try it. Please give us little more time to decide to merge it to master or not.

okdshin avatar Jul 26 '18 06:07 okdshin

Thank you so much for creating the branch so quickly. I tried it right away (last night), came across a couple of problems (explained below), but that was it. It compiled and my AWS Lambda demo app is running fine.

2 const related compile errors

The following message are warnings with the compiler on my mac, but these are errors with gcc 4.8.3 on Amazon linux:

/Users/ytakeda/Projects/Menoh/menoh/menoh/mkldnn/operator/sum.cpp:40:61: warning: 'const' qualifier on reference type 'decltype(input_dims.front())' (aka
      'std::__1::vector<int, std::__1::allocator<int> > &') has no effect [-Wignored-qualifiers]
                 [&input_dims](decltype(input_dims.front()) const& e) {
                                                            ^~~~~
/Users/ytakeda/Projects/Menoh/menoh/menoh/onnx.cpp:220:29: warning: 'const' qualifier on reference type 'decltype(* onnx_model.graph().initializer().begin())'
      (aka 'const onnx::TensorProto &') has no effect [-Wignored-qualifiers]
                            const& tensor) { return tensor.name(); });
                            ^~~~~

I simply removed the const to move on, compiled fine, but I am not fully sure if that would be your intention.

nlohmann/json do not compile with gcc < 4.9

The error occurs in this line:

#elif defined(__GNUC__) && !(defined(__ICC) || defined(__INTEL_COMPILER))
    #if (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + __GNUC_PATCHLEVEL__) < 40900
        #error "unsupported GCC version - see https://github.com/nlohmann/json#supported-compilers"
    #endif
#endif

Looking at the past issues, it appears that this module was suffering from a compiler issue with gcc earlier than 4.9. Actual compile errors were fixed, but the author thinks it only partially solves the problem (I don't know exactly what not solved), left the above gcc compiler version guard. There were two other issue tickets regarding gcc 4.8, but both of them were marked as won't fix.

It is so unfortunate because AWS Lambda's gcc is 4.8.3.

I modified 40900 to 40800 just to see, and it compiled no problem. My demo app seems to be working now.

enobufs avatar Jul 27 '18 04:07 enobufs

I would also vote for c++11 to try to be natively compatible with redhat/centos 6&7 which are still by default on gcc 4.8. Tks.

WilliamTambellini avatar Jul 27 '18 05:07 WilliamTambellini