Reading .ot (Visual Studio): tree type isn't registered therefore read() fails

[merfels]

Hi,

As discussed in the Google Groups, there seems to be an issue with Visual Studio and the File Factory pattern when trying to read in .ot trees. The expected behavior is that the tree type should be recognized when being read from file, but the static member initialization technique which should register the tree type doesn't work in this setup, which results in an unrecognized tree type when trying to load it.

System: Windows 7 64 bit, Windows SDK 7.1, Visual Studio 2010, octomap-1.6.1

Example which shows the behavior:

OcTree tree (0.01);
tree.write("simple_tree.ot");
AbstractOcTree * readTree = AbstractOcTree::read("simple_tree.ot");
if(readTree) {
OcTree * storedTree = dynamic_cast<OcTree*>(readTree);
} else {
// couldn't load tree
}

The tree can not be read in from file. The problem is that the instantiation of a new OcTree should result in the creation of the static StaticMemberInitializer ocTreeMemberInit which is defined in OcTree.h and subsequently register the tree type. Loading from file could then associate the stored file with this tree type. This seems to be an issue with the Visual Studio compiler.

One possible workaround is to put a function "void dummy() {} ;" in the class StaticMemberInitializer and calling it from within the OcTree constructor. I tried that and apparently this doesn't get optimized away, but it adds of course some bloat and is uncleaner / less comprehensible.

Best,
Christian

[ahornung]

Thanks for your detailed analysis. I agree that a cleaner solution would be preferable. What about adding volatile to static StaticMemberInitializer ocTreeMemberInit;? Another option would be adding a VS-specific pragma:

#pragma optimize( "", off )
//...
#pragma optimize( "", on)

http://msdn.microsoft.com/en-us/library/chh3fb0k.aspx

In any case, the smallest possible change that helps would be desirable since it has to be added to every derived octree class.

[merfels]

I tried adding the volatile keyword and disabling optimization but that didn't help. I struggle to find out the real cause behind this behavior so finding a good solution is hard. If you have any more ideas then I'm happy to help you.

[ahornung]

Alas, I don't really know Visual Studio much (don't even have it running here) so I don't know how to tell their compiler to behave. There should be no reason to optimize that code out in the first place. I found some similar reports on the net, that's why I was hoping that either volatile or the optimize pragma would help:
http://stackoverflow.com/questions/1229430/how-do-i-prevent-my-unused-global-variables-being-compiled-out/
http://stackoverflow.com/a/12243667
http://bytes.com/topic/net/answers/497057-static-instance-class-optimized-out-existence

[merfels]

Thanks for the additional input. I agree that the compiler should not discard that code as it clearly has side-effects and it is kind of frustrating to find the right tweak to get it to work. For now, I will probably go for my own derived tree class with the workaround I proposed earlier, which isn't the cleanest solution but works just fine.

[ahornung]

I'm inclined to close this issue. The "test_color_tree" unit test succeeds with a current MSVC (Visual Studio 2013 and 2015, Win7 x64). Since I can't reproduce the issue I'm assuming it's fixed in the compiler.

[merfels]

Closing the issue is fine with me. I am still using the proposed workaround under MSVC 2010. As I don't have MSVC 2013 or 2015, I can't confirm whether or not the test_color_tree unit test works there.

[ahornung]

Seems like that was too early, the problem still persists but manifests differently with MSVC 2013 / 2015. Now ColorOcTree works but no longer OcTree (causing test_io to fail).

Here's another pointer that could work (actually your workaround hidden in a #define):
http://stackoverflow.com/questions/599035/force-visual-studio-to-link-all-symbols-in-a-lib-file

[ahornung]

There's now a pull request with the dummy function you described: #101

Does that look OK to you, @merfels? The question is whether to make it transparent or hide it in a #define.

[merfels]

That's exactly how I implemented it and it works fine under VS 2010.

I personally prefer to not put in a #define as things get less clear with more #define's. If people want to write their own tree implementation, they need to know what this workaround does. My suggestion is to put a comment in Doxygen explaining the issue and linking to this page.

[ahornung]

Yes, I agree that it's the best to have it as clear as possible!