microsoft / DiskANN

Graph-structured Indices for Scalable, Fast, Fresh and Filtered Approximate Nearest Neighbor Search
Other
1.06k stars 215 forks source link

Use TCMalloc to fix system memory leak #494

Closed jinwei14 closed 9 months ago

jinwei14 commented 9 months ago

Asking help on cmake change for

1. "..\Boost1_67\boost_1_67_0\lib64-msvc-14.1\libboost_program_options-vc141-mt-gd-x64-1_67.lib" missing The root cause is from this line which direct to the wrong subfolder of boost https://github.com/microsoft/DiskANN/blob/87990dacfbfac421a7a4d10d703af480cd0582ae/CMakeLists.txt#L95

2. libtcmalloc_minimal.lib is auto deleted which is needed as included lib I believe think customized command https://github.com/microsoft/DiskANN/blob/87990dacfbfac421a7a4d10d703af480cd0582ae/CMakeLists.txt#L221

will cause libtcmalloc_minimal.lib not been build for the second time, Error log:

1>C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Microsoft\VC\v170\Microsoft.CppCommon.targets(254,5): warning MSB8065: Custom build for item "E:\DevRepo\DiskANN\build\CMakeFiles\6dd73d770f6e1c5cc60d5c78e08ca559\libtcmalloc_minimal.lib.rule" succeeded, but specified output "e:\devrepo\diskann\gperftools\x64\release-patch\libtcmalloc_minimal.lib" has not been created. This may cause incremental build to work incorrectly.


Reference Issues/PRs

What does this implement/fix? Briefly explain your changes.

We looked into the memory usage issue on Windows and found that calls to “ReleaseFreeMemory()” have be #defined out in the current version of the code. For instance, in pq.cpp:

if defined(RELEASE_UNUSED_TCMALLOC_MEMORY_AT_CHECKPOINTS) && defined(DISKANN_BUILD)

MallocExtension::instance()->ReleaseFreeMemory();

endif

Both RELEASE_UNUSED_TCMALLOC_MEMORY_AT_CHECKPOINTS and DISKANN_BUILD have not been defined in the preprocessor. These calls are required to instruct tcmalloc to free unused blocks of memory.

Can you please try the following:

If you build with these changes, you shouldn’t face the issue of memory consumption increasing continuously. Not sure why CMake is not adding these flags and options, but I’m on leave tomorrow and Wednesday so we thought we could unblock you while we looked into that issue.

Any other comments?

jinwei14 commented 9 months ago

he correct project file now? (Where the TCMALLOC symbo

hi @gopalrs I see there are few places with this defined(RELEASE_UNUSED_TCMALLOC_MEMORY_AT_CHECKPOINTS) && defined(DISKANN_BUILD) but comment out, what is intension of this and could you please help with it?

daxpryce commented 9 months ago

Thanks for including me on this, I'm so happy I was able to catch what would have been a catastrophic problem for the python library if I hadn't been on this review.