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

Fixes map fields' behavior when used with arena. #459

Closed
wants to merge 3 commits into from

Conversation

xfxyjwf
Copy link
Contributor

@xfxyjwf xfxyjwf commented Jun 3, 2015

  1. Implemented NewDeleteCapture to detect heap allocations.
  2. Made MapField allocate mutexs lazily so generated API will not cause any heap allocation when using arena.
  3. Added a type trait Arena::is_destructor_skippable to check whether an object's destructor can be skipped when used with arena. Removed some incorrectly marked DestructorSkippable_ typedef.

xfxyjwf added 2 commits Jun 3, 2015
Changed MapField to allocate mutex lazily and removed some incorrectly used DestructorSkippable_.
Currently the parsing code of lite messges will store unknown fields in a
string object that is not allocated in the arena. Before that bug is fixed,
heap check will fail for arena lite runtime tests.
@xfxyjwf
Copy link
Contributor Author

xfxyjwf commented Jun 3, 2015

Added a new commit to disable heap check for arena lite runtime tests. These tests will fail because of #445

@xfxyjwf
Copy link
Contributor Author

xfxyjwf commented Jun 6, 2015

@TeBoring to review

@@ -495,6 +495,20 @@ class LIBPROTOBUF_EXPORT Arena {
static const type value;
};

template<typename T>
Copy link
Contributor

@TeBoring TeBoring Jul 9, 2015

Choose a reason for hiding this comment

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

We already have this internally.

@TeBoring TeBoring added this to the 3.0.0-beta-1 milestone Jul 9, 2015
@TeBoring
Copy link
Contributor

TeBoring commented Jul 17, 2015

@xfxyjwf to reply new comments

@xfxyjwf
Copy link
Contributor Author

xfxyjwf commented Jul 17, 2015

As you have already made part of this change in our internal code base, I'll hold this PR until next down-integration when I'll merge and update the code.

@xfxyjwf xfxyjwf removed this from the 3.0.0-beta-1 milestone Aug 28, 2015
@xfxyjwf xfxyjwf closed this Jul 13, 2016
@xfxyjwf xfxyjwf deleted the skippable branch Jul 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants