Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce memory usage for raster source handling. #5572

Merged
merged 16 commits into from
Nov 14, 2019
Merged
Prev Previous commit
Next Next commit
remove unused lines
Tomonobu Saito authored and Tomonobu Saito committed Oct 3, 2019
commit e4aaf07879a6edd0d9f60d1413f14851d6a49ccf
35 changes: 2 additions & 33 deletions include/extractor/raster_source.hpp
Original file line number Diff line number Diff line change
@@ -50,13 +50,13 @@ class RasterGrid
xdim = _xdim;
ydim = _ydim;
_data.reserve(ydim * xdim);
BOOST_ASSERT(_data.capacity() >= ydim * xdim);
BOOST_ASSERT(ydim * xdim <= _data.capacity());

// Construct FileReader
storage::io::FileReader file_reader(filepath, storage::io::FileReader::HasNoFingerprint);
std::string buf;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use the whole word buffer. In previous version it was buffer.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, Thanks for your review and comment.
I will fix as your comment. (buf --> buffer)

buf.resize(xdim * 11); // INT32_MAX = 2147483647 = 10 chars + 1 white space = 11
BOOST_ASSERT(buf.size() >= xdim * 11);
BOOST_ASSERT(xdim * 11 <= buf.size());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this check relevant? Can buf.size() < xdim * 11 without exception at previous line?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your comment.
In original code, I found below code.
buffer.resized(file_reader.GetSize())
My understanding is "to expand the buffer size to same with raster file size."
By this pull request, I changed from whole size buffer to line buffer to reduce the memory usage.
So, just in case (and respect to original code), I expand the buffer size to "maximum size of one line (logically)".
Maybe this code is not needed or there is better way.
I'm glad if you could give me some advice.

Best Regards,
Tomonobu Saito.


for (unsigned int y = 0 ; y < ydim ; y++) {
// read one line from file.
@@ -74,37 +74,6 @@ class RasterGrid
}
BOOST_ASSERT(x == xdim);
}
/*
std::string buffer;
buffer.resize(file_reader.GetSize());

BOOST_ASSERT(buffer.size() > 1);

file_reader.ReadInto(&buffer[0], buffer.size());

boost::algorithm::trim(buffer);

auto itr = buffer.begin();
auto end = buffer.end();

bool r = false;
try
{
r = boost::spirit::qi::parse(
itr, end, +boost::spirit::qi::int_ % +boost::spirit::qi::space, _data);
}
catch (std::exception const &ex)
{
throw util::exception("Failed to read from raster source " + filepath.string() + ": " +
ex.what() + SOURCE_REF);
}

if (!r || itr != end)
{
throw util::exception("Failed to parse raster source: " + filepath.string() +
SOURCE_REF);
}
*/
}

RasterGrid(const RasterGrid &) = default;
28 changes: 0 additions & 28 deletions include/storage/io.hpp
Original file line number Diff line number Diff line change
@@ -70,35 +70,7 @@ class FileReader
throw;
}
}
/*
std::size_t GetSize()
{
const boost::filesystem::ifstream::pos_type position = input_stream.tellg();
input_stream.seekg(0, std::ios::end);
const boost::filesystem::ifstream::pos_type file_size = input_stream.tellg();

if (file_size == boost::filesystem::ifstream::pos_type(-1))
{
throw util::RuntimeError("Unable to determine file size for " +
std::string(filepath.string()),
ErrorCode::FileIOError,
SOURCE_REF,
std::strerror(errno));
}

// restore the current position
input_stream.seekg(position, std::ios::beg);

if (fingerprint == FingerprintFlag::VerifyFingerprint)
{
return std::size_t(file_size) - sizeof(util::FingerPrint);
}
else
{
return file_size;
}
}
*/
/* Read one line */
template <typename T> void ReadLine(T *dest, const std::size_t count) {
if (0 < count) {
16 changes: 2 additions & 14 deletions src/extractor/raster_source.cpp
Original file line number Diff line number Diff line change
@@ -95,13 +95,7 @@ int RasterContainer::LoadRasterSource(const std::string &path_string,
const auto _xmax = static_cast<std::int32_t>(util::toFixed(util::FloatLongitude{xmax}));
const auto _ymin = static_cast<std::int32_t>(util::toFixed(util::FloatLatitude{ymin}));
const auto _ymax = static_cast<std::int32_t>(util::toFixed(util::FloatLatitude{ymax}));
/*
// for debug : list up all keys and values
util::Log() << "Num of Raster Sources : " << LoadedSourcePaths.size();
for (auto i = LoadedSourcePaths.begin(); i != LoadedSourcePaths.end(); ++i) {
util::Log() << "Key : " << i->first << " Value: " << i->second;
}
*/

const auto itr = LoadedSourcePaths.find(path_string);
if (itr != LoadedSourcePaths.end())
{
@@ -130,13 +124,7 @@ int RasterContainer::LoadRasterSource(const std::string &path_string,
LoadedSources.push_back(std::move(source));

util::Log() << "[source loader] ok, after " << TIMER_SEC(loading_source) << "s";
/*
// for debug : list up all keys and values
util::Log() << "Num of Raster Sources : " << LoadedSourcePaths.size();
for (auto i = LoadedSourcePaths.begin(); i != LoadedSourcePaths.end(); ++i) {
util::Log() << "Key : " << i->first << " Value: " << i->second;
}
*/

return source_id;
}