Closed alex85k closed 8 years ago
I am looking through your changes, here are some comments:
Thank you very much for your time and review!
I'll try to answer the questions here.
Complex build system for header-only library is not actually needed. The reason to use it was the same as makefiles - to provide examples for cmake-based projects that use Osmium and automate compilation of examples and test on system that does not have Unix-compatible make and libraries in standard paths.
Here is the bechmark of storages for small file RU-BA.osm.pbf (24Mb), example osmium_mpdump.exe (changed typedef storage_sparsetable_t):
SparseTable 117s 830-915Mb Vector 112s 100Mb SparseMap 126s 114Mb SimpleMap 115s 247Mb
I ignored the Vector class when adding my classes, because I did not remember that the input was sorted by ID. Maybe map storages are not needed after all.
The same file on Ubuntu (same machine) is processed 3-4 times faster: SparseTable 31s 670 Mb Vector 28s 115 Mb SparseMap 40.5s 112 Mb SimpleMap 30 s 250 Mb (MinGW is slow or maybe some library is built without optimization. Visual Studio variant runs as fast as Linux or faster)
Visual Studio comaptible build is ready. I have removed redundant inttypes.h and boost:: and placed getopt into separate library on Windows. (Leaving gettimeofday and mmap in separate osmium/windows folder for now).
I am not sure if simple_table and simple_map sta are needed in master branch - Vector seems to be faster (no files with unsorted IDs?).
CMake files can be generated in separate folder (if user chooses it), so original makefiles remain untouched as "standard" build system.
Can you have a look at the code once again?
I have taken out some non-controversial bits and pieces from your changes (some of them as is, some slightly changed) and committed them in 8f77d4e. But that were the easy parts...
The rest of the stuff is somewhat difficult to handle, one reason is that there are several issues mixed up: cmake, general windows support, storage variantes, ...
Can you move all the cmake stuff into it's own directory? I can't test and maintain it, so until I am sure that somebody is actually maintaining this in the long term, I'd rather not have it mixed up with everything else.
I'll have a look at the gettimeofday stuff and see whether we actually need that.
Can we pull in the mmap stuff from some other project as a sub-module or something?
I have added some changes in the last commits that should fix some more of the problems. Can you try that and then rebase your changes to the current version to see what else needs to be done.
The progress handler is not important enough to warrant lots of special windows stuff. We should just document it as unix-only. This way we don't have to do the gettimeofdays stuff.
Ok, tomorrow I'll check and rebase the changes, separating compatibility from all other additions.
Thank you! I have checked the changes and merged them to my mingw branch (do not know exactly how to use rebase and if it is suitable).
Currently, no examples compile on MinGW because of osmfile.hpp issue - sys/wait (fork, pipe, pid_t, ,,, are not supported on Windows except maybe Cygwin emulation).
If it is patched ( https://dl.dropboxusercontent.com/u/63393258/mingw_min.patch ) then all tests and most of examples can be compiled and they pass OK.
To compile all examples we need one of following:
We also need to solve problem with progress.hpp (maybe implement it in some cross-platform way?) or drop its support on Windows. Maybe using simple clock() is better (with ifdefs, if needed)?
To allow compilation of headers in VS in other projects, we need some extra patching:
CMake is needed to generate and run Visual Studio projects for examples and tests and is optional (I am improving and separting it to always keep existing makefiles untouched). Makefile theoretically can be used under Mingw+Msys, but it is very hard to make working system with gdal-config etc. (simpler to patch makefile by hand, hardcoding libraries and include dirs).
I have updated my sources and made them windows-compatible again. Here are the changes we need to make the most headers windows-compatible: https://github.com/alex85k/osmium/commit/e486c4988791b0447565f40a73490f55a7986b5e
And here are the cmake scripts (building with main makefile overwriting is prohibited now): https://github.com/alex85k/osmium/commit/7fda597dba7eb25c9539a6fcddbbe7cceca12de1
I am currently working on Windows compatibility of Osmium in my fork. The goal is to support MinGW 32/64 and MSVC. (Certain features can be disabled by #ifdefs before someone rewrite them in native code).
Currently I have CMake build scripts and slightly patched code working at least for Mingw32 and Ubuntu, planning also to support Visual Studio. Only disabled features for now is execute() method in osmfile.hpp and mmap_anon storage.
CMake file support installation and automatic testing, missig features for now are valgrind and cppcheck.
Can you please give me some advice on how to make the code suitable for future inclusion? https://github.com/alex85k/osmium/tree/mingw
For example, CMake generates the makefiles that overwrite existing ones. They are not to be committed (we want to keep old building system too).