Tim Armstrong has posted comments on this change. Change subject: IMPALA-3200: Implement suballocator for splitting buffers ......................................................................
Patch Set 4: (3 comments) Sorry for the slow turnaround, got caught up in things. http://gerrit.cloudera.org:8080/#/c/4715/4//COMMIT_MSG Commit Message: Line 14: larger chunks. This helps avoid fragmentation and is quite effective > I think there are a few ways to get both what I am concerned about (buddy a Gotcha. I added the static_assert - that is a good idea. I think the Atom idea is nice but I think we end up with the same problem with large buffers, since the buffer pool only supports powers-of-two. http://gerrit.cloudera.org:8080/#/c/4715/11/be/src/bufferpool/suballocator-test.cc File be/src/bufferpool/suballocator-test.cc: Line 107: // Make allocations smaller than the buffer size. > What happened to the const? The const was incorrect - I pushed a patchset without the fix. The problem is that GetUsedReservation() is a non-const method since it acquires the mutex. http://gerrit.cloudera.org:8080/#/c/4715/4/be/src/bufferpool/suballocator.h File be/src/bufferpool/suballocator.h: Line 171: Suballocation* prev_free_; > Just so I'm clear, in the current system, a Suballocation may have several Yep, that's true. I realise that it's really not obvious why the lifetime of 'buddy_' and 'prev_free_' is shorter than the unique_ptr. I documented that and then described the ownership cases in the class header. -- To view, visit http://gerrit.cloudera.org:8080/4715 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8bfe0e429f67ad273f7c7d0816703a9e6c3da788 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
