>>> I received a bad alloc when uploading a large file with CTest. The patch
>>> below resolved this.
I just took a look at the code and noticed that it is quite memory
consumption heavy ((2 * encoded_buffer_size) + file_buffer_size).
This implementation could be used instead (not tested):
std::string cmCTest::Base64EncodeFile(const std::string& file)
{
const size_t len = cmSystemTools::FileLength(file);
cmsys::ifstream ifs(file.c_str(), std::ios::in
#ifdef _WIN32
| std::ios::binary
#endif
);
std::string base64; // set to empty string by default
// section for throwing encoded_buffer out of scope as soon as possible to
// use up less memory
{
const size_t output_size = ((len - 1) / 3) * 4 + 4;
const size_t final_size = output_size + (output_size / 64) * 2;
std::vector<unsigned char> encoded_buffer;
encoded_buffer.resize(final_size);
// section for throwing file_buffer out of scope as soon as possible to
// use up less memory
{
std::vector<unsigned char> file_buffer;
file_buffer.resize(len);
ifs.read(reinterpret_cast<char*>(file_buffer), len);
ifs.close();
unsigned long rlen
= cmsysBase64_Encode(&file_buffer[0], len, &encoded_buffer[0], 1);
}
// consume less memory by swapping tmp string buffer with base64 variable
std::string(encoded_buffer.begin(), encoded_buffer.begin() +
rlen).swap(base64);
}
return base64; // this will produce another copy in C++98
}
It lowers memory consumption to approximately (2 *
encoded_buffer_size) and it also makes memory deallocation exception
safe (file_buffer and encoded_buffer variables).
A better solution would be to rewrite the function to:
void cmCTest::Base64EncodeFile(const std::string& file, std::string&
base64) to prevent one more copy at return but since this changes the
interface we could go a step further and simply rewrite the entire
function and cmsysBase64_Encode function to use streams and thereby
lower the memory consumption even further (less copying between
different containers and more readable than the above version).
Thoughts?
p.s. I think that using std::ios::binary would work on all platforms
so I guess that the function does not need ifdef at the top.
Regards,
Domen
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
