{"action":"created","issue":{"url":"https://api.github.com/repos/ceph/ceph/issues/25077","repository_url":"https://api.github.com/repos/ceph/ceph","labels_url":"https://api.github.com/repos/ceph/ceph/issues/25077/labels{/name}","comments_url":"https://api.github.com/repos/ceph/ceph/issues/25077/comments","events_url":"https://api.github.com/repos/ceph/ceph/issues/25077/events","html_url":"https://github.com/ceph/ceph/pull/25077","id":380265760,"node_id":"MDExOlB1bGxSZXF1ZXN0MjMwNTA1NDU0","number":25077,"title":"common: drop append_buffer from bufferlist. Use simple carriage instead","user":{"login":"rzarzynski","id":1891217,"node_id":"MDQ6VXNlcjE4OTEyMTc=","avatar_url":"https://avatars2.githubusercontent.com/u/1891217?v=4","gravatar_id":"","url":"https://api.github.com/users/rzarzynski","html_url":"https://github.com/rzarzynski","followers_url":"https://api.github.com/users/rzarzynski/followers","following_url":"https://api.github.com/users/rzarzynski/following{/other_user}","gists_url":"https://api.github.com/users/rzarzynski/gists{/gist_id}","starred_url":"https://api.github.com/users/rzarzynski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rzarzynski/subscriptions","organizations_url":"https://api.github.com/users/rzarzynski/orgs","repos_url":"https://api.github.com/users/rzarzynski/repos","events_url":"https://api.github.com/users/rzarzynski/events{/privacy}","received_events_url":"https://api.github.com/users/rzarzynski/received_events","type":"User","site_admin":false},"labels":[{"id":199292951,"node_id":"MDU6TGFiZWwxOTkyOTI5NTE=","url":"https://api.github.com/repos/ceph/ceph/labels/common","name":"common","color":"006b75","default":false},{"id":204137300,"node_id":"MDU6TGFiZWwyMDQxMzczMDA=","url":"https://api.github.com/repos/ceph/ceph/labels/performance","name":"performance","color":"bfdadc","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-11-13T14:57:51Z","updated_at":"2018-11-15T15:16:34Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/ceph/ceph/pulls/25077","html_url":"https://github.com/ceph/ceph/pull/25077","diff_url":"https://github.com/ceph/ceph/pull/25077.diff","patch_url":"https://github.com/ceph/ceph/pull/25077.patch"},"body":"Summary\r\n=======\r\nWhen handling appends `bufferlist` tends to allocate memory in bigger chunks (4 KB) to amortize the costs across multiple calls and avoid fragmentation. This strategy imposes a *tracking mechanism* maintaining the writable space over a sequence of `::appends`, possibly a mix of C-string and `buffer::ptr`-taking variants.\r\n\r\nThis mechanism is currently implemented as `buffer::ptr`-typed `append_buffer`. Appending a C-string typically results in `memcpy` and a call to `::append(const buffer::ptr&, unsigned off, unsigned len)` where a try to merge the new segment (freshly cut off from `append_buffer`) with `_buffers.back()` takes place. However, this has some drawbacks:\r\n* the extra call, stack canary management and several comparisons/branches needed for *merging* might be just too much when appending a 4-byte integer. Using so tiny granularity is not unusual because of the old style encoding.\r\n* Higher average `nref` value of underlying `buffer::raw` due to being referenced by at least two `buffer::ptr` instances: `append_buffer` and a node stored on the `_buffers` list. This translates into more reference counting (calls to `ptr::ptr(const ptr&)` and `ptr::release()`) and increased  pressure on memory allocator as a hypercombined `buffer::raw` has room only for single `buffer::ptr`.\r\n\r\nThe pull request tries to deal with these problems by making `::appends` to operate directly on *last writable* node of `_buffers` tracked by a dump pointer  called`_carriage`. It also simplifies `contiguous_appender` and enables further optimizations (`contiguous_reserver`).\r\n\r\nMicrobenchmarking\r\n===============\r\n4-bytes appends using the C-string variant of `bufferlist::append()` take `1 - 10.339933 / 16.662740 ~= 0.38` less than before the changes.\r\n\r\nReference point: eeb485f2a9e757d5d0a70fe9b26b423123e5c4dd\r\n----------------------------------------------------------------------\r\nThe changeset sits on top of the [hypercombined bufferlist changes](https://github.com/ceph/ceph/pull/24882). The reference points (eeb485f2a9e757d5d0a70fe9b26b423123e5c4dd) is\r\n`include: buffer_raw.h uses C++11-compatible std::aligned_storage. `\r\n```\r\n$ ./bin/unittest_bufferlist --gtest_filter=BufferList.append_bench\r\nNote: Google Test filter = BufferList.append_bench\r\n[==========] Running 1 test from 1 test case.\r\n[----------] Global test environment set-up.\r\n[----------] 1 test from BufferList\r\n[ RUN      ] BufferList.append_bench\r\n4000 fills of buffer len 1048576 with 4 byte appends in 16.662740\r\n4000 fills of buffer len 1048576 with 16 byte appends in 4.769071\r\n4000 fills of buffer len 1048576 with 64 byte appends in 1.685499\r\n4000 fills of buffer len 1048576 with 256 byte appends in 0.834929\r\n4000 fills of buffer len 1048576 with 1024 byte appends in 0.524226\r\n4000 fills of buffer len 1048576 with 4096 byte appends in 0.479762\r\n4000 fills of buffer len 1048576 with 16384 byte appends in 0.275694\r\n[       OK ] BufferList.append_bench (25232 ms)\r\n[----------] 1 test from BufferList (25232 ms total)\r\n\r\n[----------] Global test environment tear-down\r\n[==========] 1 test from 1 test case ran. (25232 ms total)\r\n[  PASSED  ] 1 test.\r\n```\r\n\r\nAfter: 26378057d4407506847f8f3268a5855b4c01fed5\r\n----------------------------------------------------------------------\r\n```\r\n$ ./bin/unittest_bufferlist --gtest_filter=BufferList.append_bench\r\nNote: Google Test filter = BufferList.append_bench\r\n[==========] Running 1 test from 1 test case.\r\n[----------] Global test environment set-up.\r\n[----------] 1 test from BufferList\r\n[ RUN      ] BufferList.append_bench\r\n4000 fills of buffer len 1048576 with 4 byte appends in 10.339933\r\n4000 fills of buffer len 1048576 with 16 byte appends in 3.176103\r\n4000 fills of buffer len 1048576 with 64 byte appends in 1.359964\r\n4000 fills of buffer len 1048576 with 256 byte appends in 0.736537\r\n4000 fills of buffer len 1048576 with 1024 byte appends in 0.506640\r\n4000 fills of buffer len 1048576 with 4096 byte appends in 0.460884\r\n4000 fills of buffer len 1048576 with 16384 byte appends in 0.262999\r\n[       OK ] BufferList.append_bench (16844 ms)\r\n[----------] 1 test from BufferList (16844 ms total)\r\n\r\n[----------] Global test environment tear-down\r\n[==========] 1 test from 1 test case ran. (16844 ms total)\r\n[  PASSED  ] 1 test.\r\n```\r\n"},"comment":{"url":"https://api.github.com/repos/ceph/ceph/issues/comments/439075388","html_url":"https://github.com/ceph/ceph/pull/25077#issuecomment-439075388","issue_url":"https://api.github.com/repos/ceph/ceph/issues/25077","id":439075388,"node_id":"MDEyOklzc3VlQ29tbWVudDQzOTA3NTM4OA==","user":{"login":"rzarzynski","id":1891217,"node_id":"MDQ6VXNlcjE4OTEyMTc=","avatar_url":"https://avatars2.githubusercontent.com/u/1891217?v=4","gravatar_id":"","url":"https://api.github.com/users/rzarzynski","html_url":"https://github.com/rzarzynski","followers_url":"https://api.github.com/users/rzarzynski/followers","following_url":"https://api.github.com/users/rzarzynski/following{/other_user}","gists_url":"https://api.github.com/users/rzarzynski/gists{/gist_id}","starred_url":"https://api.github.com/users/rzarzynski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rzarzynski/subscriptions","organizations_url":"https://api.github.com/users/rzarzynski/orgs","repos_url":"https://api.github.com/users/rzarzynski/repos","events_url":"https://api.github.com/users/rzarzynski/events{/privacy}","received_events_url":"https://api.github.com/users/rzarzynski/received_events","type":"User","site_admin":false},"created_at":"2018-11-15T15:16:34Z","updated_at":"2018-11-15T15:16:34Z","author_association":"CONTRIBUTOR","body":"CC: @rouming."}}
{"actor":{"display_login":"rzarzynski"}}