Fix 'attempting to reference a deleted function' with MSVC

[cloudhan]

Fixed #8192, Close #8193

This PR will suppress #8193.

noexcept(true) is not required for std::unordered_map, causing compilation error with MSVC for noexcept moveable BackwardBuilder. For more detail, please refers to microsoft/STL#165

So a simple fix is just replace std::unordered_map with noexcept guaranteed hash map

Note, according to https://www.ece.uvic.ca/~frodo/cppdraft/n4659/html/dcl.fct.def#delete-2

A program that refers to a deleted function implicitly or explicitly, other than to declare it, is ill-formed. [ Note: This includes calling the function implicitly or explicitly and forming a pointer or pointer-to-member to the function. It applies even for references in expressions that are not potentially-evaluated. If a function is overloaded, it is referenced only if the function is selected by overload resolution. The implicit odr-use ([basic.def.odr]) of a virtual function does not, by itself, constitute a reference. — end note ]

Deleted move assignment operator is still used, causing the program ill-formed, but seems both GCC and MSVC accept it.

[cloudhan]

@niboshi remove noexcept might also do the trick, might need additional minor changes on the call site of move. Is it OK to swap std::unordered_map for absl::flat_hash_map for noexcept-ness?

[emcastillo]

Just for reference "Iteration on absl's tables is O(capacity), not O(size)."

Can we measure how this change could impact performance?

[niboshi]

Another way around could be to try { } catch (...) { } ?

[cloudhan]

Another way around could be to try { } catch (...) { } ?

Only if you want to implement move ctor manually.

[cloudhan]

FYI, there are interesting sets of benchmarks about hashmap https://martin.ankerl.com/2019/04/01/hashmap-benchmarks-01-overview/

[emcastillo]

I will try to measure the effect on chainerX, from the links you posted, it actually seems that we could get more performance!!

[cloudhan]

Actually only

chainer/chainerx_cc/chainerx/backward_builder.h

Line 225 in 4c79f50

absl::flat_hash_map<BackpropId, std::shared_ptr<internal::OpNode>> op_node_map_;

is needed, if you don't mind using std and abseil's hash map simultaneously.

[cloudhan]

@emcastillo @niboshi Any progress or suggestion?

[emcastillo]

Sorry, I will look at it ASAP

[emcastillo]

I just tested it with imagenet and resnet-50, performance remains exactly the same 👍

[emcastillo]

Lets fix Travis errors

travis issues

[emcastillo]

Jenkins, test this please

[chainer-ci]

Jenkins CI test (for commit 4c78844, target branch master) succeeded!

[emcastillo]

Jenkins, test this please

[chainer-ci]

Jenkins CI test (for commit 471e82c, target branch master) succeeded!