Re: [cmake-developers] [PATCH] Avoid bad alloc for large files
Am Montag, 22. Dezember 2014, 22:16:50 schrieb Rolf Eike Beer: Am Montag, 22. Dezember 2014, 10:50:28 schrieb Brad King: On 12/20/2014 08:44 AM, Rolf Eike Beer wrote: This is basically the same, but it avoids the needless floating point arithmetic. Does it work for you? Thanks, Eike. Please add a topic to put this in 'next' when ready. Done. I wonder if we should not check for new returning null? Ok, now I get this: cmCTest.cxx C:\Dashboards\My Tests\CMakeNext-vs12-64-ninja-src\Source\cmCTest.cxx(1705) : warning C4267: 'argument' : conversion from 'size_t' to 'unsigned long', possible loss of data The problem is: this is absolutely right. On Windows64 this means that the maximum file size will be 4GB, or one needs to change the base64 implementation. Which way to go? Eike signature.asc Description: This is a digitally signed message part. -- 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
Re: [cmake-developers] [PATCH] Avoid bad alloc for large files
Am Dienstag, 23. Dezember 2014, 10:27:47 schrieb Rolf Eike Beer: Am Montag, 22. Dezember 2014, 22:16:50 schrieb Rolf Eike Beer: Am Montag, 22. Dezember 2014, 10:50:28 schrieb Brad King: On 12/20/2014 08:44 AM, Rolf Eike Beer wrote: This is basically the same, but it avoids the needless floating point arithmetic. Does it work for you? Thanks, Eike. Please add a topic to put this in 'next' when ready. Done. I wonder if we should not check for new returning null? Ok, now I get this: cmCTest.cxx C:\Dashboards\My Tests\CMakeNext-vs12-64-ninja-src\Source\cmCTest.cxx(1705) : warning C4267: 'argument' : conversion from 'size_t' to 'unsigned long', possible loss of data The problem is: this is absolutely right. On Windows64 this means that the maximum file size will be 4GB, or one needs to change the base64 implementation. Which way to go? *lol* From: Brad King (Code Review) rev...@kitware.com Date: Tue, 23 Dec 2014 04:27:55 -0500 Subject: Change in KWSys[master]: Base64: Use size_t for lenghts in API signature.asc Description: This is a digitally signed message part. -- 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
Re: [cmake-developers] [PATCH] Avoid bad alloc for large files
On 12/23/2014 4:27 AM, Rolf Eike Beer wrote: cmCTest.cxx C:\Dashboards\My Tests\CMakeNext-vs12-64-ninja-src\Source\cmCTest.cxx(1705) : warning C4267: 'argument' : conversion from 'size_t' to 'unsigned long', possible loss of data The problem is: this is absolutely right. On Windows64 this means that the maximum file size will be 4GB, or one needs to change the base64 implementation. Which way to go? The KWSys Base64 API should be fixed to use size_t: http://review.source.kitware.com/18579 Domen's suggestion elsewhere in this thread of using a streaming implementation will be necessary to support large files on 32-bit platforms. That can be done as follow-up work though. -Brad -- 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
Re: [cmake-developers] [PATCH] Avoid bad alloc for large files
On 12/20/2014 08:44 AM, Rolf Eike Beer wrote: This is basically the same, but it avoids the needless floating point arithmetic. Does it work for you? Thanks, Eike. Please add a topic to put this in 'next' when ready. Thanks, -Brad -- 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
Re: [cmake-developers] [PATCH] Avoid bad alloc for large files
I received a bad alloc when uploading a large file with CTest. The patch below resolved this. Your patch is line-wrapped and can't be applied. However, I did this by hand. This is basically the same, but it avoids the needless floating point arithmetic. Does it work for you? snip std::string cmCTest::Base64EncodeFile(std::string file) snip -static_castdouble(len) * 1.5 + 5.0) ]; += new unsigned char [ (len * 3) / 2 + 5 ]; snip I came across a similar issue a few days ago in our code base (bad alloc when compressing a large file in-memory with 256 MB data ulimit per process) and I used a different formula to calculate the maximum buffer size: http://stackoverflow.com/questions/1533113/calculate-the-size-to-a-base-64-encoded-message I used a bit modified answer from kanaka. size_t output_size = ((len - 1) / 3) * 4 + 4; size_t final_size = output_size + (output_size / 64) * 2; // 64 instead of 76 since RFC 3548 and RFC 4648 allow CLRF line breaks every 64 characters This formula would give less memory allocation overhead. 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
Re: [cmake-developers] [PATCH] Avoid bad alloc for large files
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::vectorunsigned 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::vectorunsigned char file_buffer;
file_buffer.resize(len);
ifs.read(reinterpret_castchar*(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
Re: [cmake-developers] [PATCH] Avoid bad alloc for large files
Am Montag, 22. Dezember 2014, 10:50:28 schrieb Brad King: On 12/20/2014 08:44 AM, Rolf Eike Beer wrote: This is basically the same, but it avoids the needless floating point arithmetic. Does it work for you? Thanks, Eike. Please add a topic to put this in 'next' when ready. Done. I wonder if we should not check for new returning null? Eike signature.asc Description: This is a digitally signed message part. -- 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
Re: [cmake-developers] [PATCH] Avoid bad alloc for large files
Justin Borodinsky wrote:
I received a bad alloc when uploading a large file with CTest. The patch
below resolved this.
Your patch is line-wrapped and can't be applied. However, I did this by hand.
This is basically the same, but it avoids the needless floating point
arithmetic. Does it work for you?
diff --git a/Source/cmCTest.cxx b/Source/cmCTest.cxx
index 2bf7b77..1a7bf45 100644
--- a/Source/cmCTest.cxx
+++ b/Source/cmCTest.cxx
@@ -1688,7 +1688,7 @@ std::string cmCTest::Base64GzipEncodeFile(std::string
file)
//--
std::string cmCTest::Base64EncodeFile(std::string file)
{
- long len = cmSystemTools::FileLength(file);
+ const size_t len = cmSystemTools::FileLength(file);
cmsys::ifstream ifs(file.c_str(), std::ios::in
#ifdef _WIN32
| std::ios::binary
@@ -1699,8 +1699,7 @@ std::string cmCTest::Base64EncodeFile(std::string file)
ifs.close();
unsigned char *encoded_buffer
-= new unsigned char [ static_castint(
-static_castdouble(len) * 1.5 + 5.0) ];
+= new unsigned char [ (len * 3) / 2 + 5 ];
unsigned long rlen
= cmsysBase64_Encode(file_buffer, len, encoded_buffer, 1);
Eike
--
signature.asc
Description: This is a digitally signed message part.
--
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
Re: [cmake-developers] [PATCH] Avoid bad alloc for large files
Yes, thank you.
On Saturday, December 20, 2014, Rolf Eike Beer e...@sf-mail.de wrote:
Justin Borodinsky wrote:
I received a bad alloc when uploading a large file with CTest. The patch
below resolved this.
Your patch is line-wrapped and can't be applied. However, I did this by
hand.
This is basically the same, but it avoids the needless floating point
arithmetic. Does it work for you?
diff --git a/Source/cmCTest.cxx b/Source/cmCTest.cxx
index 2bf7b77..1a7bf45 100644
--- a/Source/cmCTest.cxx
+++ b/Source/cmCTest.cxx
@@ -1688,7 +1688,7 @@ std::string
cmCTest::Base64GzipEncodeFile(std::string file)
//--
std::string cmCTest::Base64EncodeFile(std::string file)
{
- long len = cmSystemTools::FileLength(file);
+ const size_t len = cmSystemTools::FileLength(file);
cmsys::ifstream ifs(file.c_str(), std::ios::in
#ifdef _WIN32
| std::ios::binary
@@ -1699,8 +1699,7 @@ std::string cmCTest::Base64EncodeFile(std::string
file)
ifs.close();
unsigned char *encoded_buffer
-= new unsigned char [ static_castint(
-static_castdouble(len) * 1.5 + 5.0) ];
+= new unsigned char [ (len * 3) / 2 + 5 ];
unsigned long rlen
= cmsysBase64_Encode(file_buffer, len, encoded_buffer, 1);
Eike
--
--
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
