efficient / libcuckoo

A high-performance, concurrent hash table
Other
1.62k stars 275 forks source link

Add Microsoft Visual Studio projects. Removal of platform/compiler dependent code (replaced with c++11 STL code). #23

Closed davidalbertonogueira closed 8 years ago

davidalbertonogueira commented 8 years ago

Native support for MSVC v140 compiler (distributed in _MSC_VER == 1900 (Visual Studio 2015)).

Replacement of platform/compiler dependent code with STL C++11 equivalent calls (examples: fix sleep() method with std::this_thread::sleep for(); add std::chrono to fix dependency on time structs; fix dependent number of threads call with std::thread::hardware_concurrency(), etc...).

Compilation in Ubuntu/Windows : ok Tests in Ubuntu/Windows : ok

mdimura commented 8 years ago

I was using libcuckoo on windows/mingw64 by building it on linux and simply including the .h files to the project as a header-only library. Of course, I did not use the city-hash, so it worked. Proper windows build would be, of course, a much cleaner solution. Strongly support this PR.

manugoyal commented 8 years ago

Hi David, thanks for the contribution! Sorry for the late response. It seems like there are a lot of extra visual-studio-specific files and changes, as well as a lot of formatting changes that made it into the diff. Would you be able to go through and remove anything nonessential? I assume only changes to the cuckoohash_map.hh, cuckoohash_util.hh, and perhaps the unit test / benchmark files are necessary to make it windows compatible, or are some of the other additions necessary to make it work?

davidalbertonogueira commented 8 years ago

Let me go over that during the weekend and I will get back to you. I will start from merging with your recent changes and I can probably offer two branches for you to merge pull, one with the code changes and another with the Microsoft Visual Studio files.

davidalbertonogueira commented 8 years ago

I've already merged and resolved the conflicts with your recent changes. If you still think it is relevant, I will try to reformat the files to reduce the number of changed lines to your version.

manugoyal commented 8 years ago

thanks! if you could reduce the number of changed lines to specific code changes in the original files, that'd be great.

davidalbertonogueira commented 8 years ago

Done. If you ignore the Windows files (Visual Studio), in the source code only the relevant changes are committed. I've also merged with your recent changes.

DJuego commented 8 years ago

Thanks!

I am interested in high performance concurrent containers. libcuckoo has very good references. ;-) However i need support for the big c/c++ compilers (msvc, gcc, clang). :-/

Please, please, please? :-}

DJuego

manugoyal commented 8 years ago

Thanks, this has been fixed by #26, #27, and #28. The library should now compile with Visual Studio 2015.

davidalbertonogueira commented 8 years ago

However, you are not providing the Visual Studio solution/project files (the Visual Studio "makefiles"). I would recommend providing them also to broaden the cardinality of possible future users.

davidalbertonogueira commented 8 years ago

Moreover, in the code you still have substings that are not valid/compiled in windows: like "sysconf(_SC_NPROCESSORS_ONLN)" and "attribute((aligned(64)));". Such instructions must be protected with "#ifndef _WIN32" macros.

Such corrections are presented in this pull request.

manugoyal commented 8 years ago

Yes thanks, I see we still have a few instances of sysconf(SC_NPROCESSORS_ONLN) in the benchmarks, so I'll address that. I think we've managed to guard all uses of __attribute_((aligned(64))) with the proper guards (see cuckoohash_util.hh).

Regarding the Visual Studio project files, I don't think they are really in the scope of the library, as we're trying to include just the files that are needed across all platforms. Furthermore, I imagine these extra files might vary from project to project, since each user might want to integrate the library in a different way.