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

Low memory vtile decoding / replace google libprotobuf with protozero #505

Merged
merged 23 commits into from
Sep 10, 2015

Conversation

springmeyer
Copy link
Member

Implements plan described in #422

Key context: #474

Before merging:

  • start manually integrating some of Implement gzip compression in the C++ layer #456 into this branch
  • test/branch some modules depending on vtile like tilelive-bridge and tilelive-vector (will require usage updates to avoid vt.parse())
  • improve code coverage

@springmeyer springmeyer added this to the v3.5.0 milestone Aug 12, 2015
@joto
Copy link

joto commented Aug 19, 2015

The code quite often has the pattern:

protozero::pbf_reader message(...);
while (message.next()) {
    if (message.tag() == N) {
        ...
    } else {
        message.skip();
    }
}

This should be replaced by the equivalent, but much simpler:

protozero::pbf_reader message(...);
while (message.next( N )) {
    ....
}

Also, for convenience, pbf_reader can be instantiated with a std::string. pbf_reader message(str.c_str(), str.size()); is the same as the shorter pbf_reader message(str);.

This overload of the constructor is also sometimes useful: pbf_reader(std::pair<const char *, size_t> data) noexcept;

Looking at the pbf_get_layer function I am thinking it would be a lot cleaner if it could just return the message. We could enable this by giving pbf_reader an operator bool() which would basically do the same thing as next(). Does this make sense?

@springmeyer
Copy link
Member Author

thanks @joto - made improvements in a013732

Dane Springmeyer added 5 commits September 8, 2015 16:38
 - hit other parse throw by calling funtion that calls get_tile
 - ensure coverage of multi_point by ensuring points are not filtered and actual multipoint is returned
@springmeyer
Copy link
Member Author

Okay, we're now 100% (https://coveralls.io/builds/3518078), so this is ready to merge.

flippmoke added a commit that referenced this pull request Sep 10, 2015
Low memory vtile decoding / replace google libprotobuf with protozero
@flippmoke flippmoke merged commit 04b7d89 into master Sep 10, 2015
@flippmoke flippmoke deleted the lazy-pbf branch September 10, 2015 21:56
@springmeyer springmeyer modified the milestones: v3.4.6, v3.5.0 Sep 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants