Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider switching to c++11 #58

Open
enobufs opened this issue Jul 26, 2018 · 3 comments
Open

Consider switching to c++11 #58

enobufs opened this issue Jul 26, 2018 · 3 comments

Comments

@enobufs
Copy link

enobufs commented Jul 26, 2018

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.

@okdshin
Copy link
Contributor

okdshin commented Jul 26, 2018

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.

@enobufs
Copy link
Author

enobufs commented Jul 27, 2018

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.

@WilliamTambellini
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants