{"action":"created","issue":{"url":"https://api.github.com/repos/golang/snappy/issues/21","labels_url":"https://api.github.com/repos/golang/snappy/issues/21/labels{/name}","comments_url":"https://api.github.com/repos/golang/snappy/issues/21/comments","events_url":"https://api.github.com/repos/golang/snappy/issues/21/events","html_url":"https://github.com/golang/snappy/pull/21","id":122892534,"number":21,"title":"Integrated sync.Pool + Multiple Optimizations","user":{"login":"AlasdairF","id":8223641,"avatar_url":"https://avatars.githubusercontent.com/u/8223641?v=3","gravatar_id":"","url":"https://api.github.com/users/AlasdairF","html_url":"https://github.com/AlasdairF","followers_url":"https://api.github.com/users/AlasdairF/followers","following_url":"https://api.github.com/users/AlasdairF/following{/other_user}","gists_url":"https://api.github.com/users/AlasdairF/gists{/gist_id}","starred_url":"https://api.github.com/users/AlasdairF/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/AlasdairF/subscriptions","organizations_url":"https://api.github.com/users/AlasdairF/orgs","repos_url":"https://api.github.com/users/AlasdairF/repos","events_url":"https://api.github.com/users/AlasdairF/events{/privacy}","received_events_url":"https://api.github.com/users/AlasdairF/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2015-12-18T06:57:14Z","updated_at":"2016-02-02T09:50:59Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/golang/snappy/pulls/21","html_url":"https://github.com/golang/snappy/pull/21","diff_url":"https://github.com/golang/snappy/pull/21.diff","patch_url":"https://github.com/golang/snappy/pull/21.patch"},"body":"I've made the following optimizations:\r\n\r\n1. sync.Pool is now used to avoid reallocating buffers for both Reads and Writes. This should drastically speed up all cases where snappy is used more than once within a reasonable time. There is very little disadvantage, even if the buffers are never reused and always expire from the pool; the overhead for that is negligible.\r\n\r\n2. I prepared the header slice of bytes to avoid it being recalculated every time.\r\n\r\n3. I added additional constants to avoid runtime calculations.\r\n\r\n4. I reduced the number of writes to the underlying writer from 2 to 1 on an average write by including the chunk header in the same slice of bytes as the chunk body (uncompressed chunks are still written with 2 writes, but compressed chunks are now written in 1 write.) In a normal case this would half the number of writes to the underlying writer. Writes can be expensive, especially to disk, so this should give a significant performance increase.\r\n\r\n5. I added a buffer on the Writer which is used to group small writes into one larger write. Writes that are already over half of the max chunk size are written directly without copying to the buffer. Small writes are copied to the buffer and written together as one chunk. This was done carefully so as not to give a performance decrease in any circumstance: only small writes are buffered, large writes are written directly, and a combination of small and large writes will work fine. This feature should enhance both write speed and compression for many small writes without any decrease in performance for large writes.\r\n\r\n6. I added the function WriteOnce, which makes encoding a single slice of bytes with snappy more efficient. This is the same but much faster than NewWriter -> Write -> Close.\r\n\r\n7. I made a number of minor optimizations, including separating Encode into two functions, which saves some redundant calculations if encode is called via the Write function.\r\n\r\nNote on compatibility: due to the addition of the buffer on the Writer it is now necessary to Close() the writer. This makes this version of snappy incompatible with the previous version. However, the performance gain is significant and so I believe it is reasonable to expect people to update their code. The Reader does not have to be closed, but not doing so will mean the buffers are not repooled."},"comment":{"url":"https://api.github.com/repos/golang/snappy/issues/comments/178480845","html_url":"https://github.com/golang/snappy/pull/21#issuecomment-178480845","issue_url":"https://api.github.com/repos/golang/snappy/issues/21","id":178480845,"user":{"login":"AlasdairF","id":8223641,"avatar_url":"https://avatars.githubusercontent.com/u/8223641?v=3","gravatar_id":"","url":"https://api.github.com/users/AlasdairF","html_url":"https://github.com/AlasdairF","followers_url":"https://api.github.com/users/AlasdairF/followers","following_url":"https://api.github.com/users/AlasdairF/following{/other_user}","gists_url":"https://api.github.com/users/AlasdairF/gists{/gist_id}","starred_url":"https://api.github.com/users/AlasdairF/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/AlasdairF/subscriptions","organizations_url":"https://api.github.com/users/AlasdairF/orgs","repos_url":"https://api.github.com/users/AlasdairF/repos","events_url":"https://api.github.com/users/AlasdairF/events{/privacy}","received_events_url":"https://api.github.com/users/AlasdairF/received_events","type":"User","site_admin":false},"created_at":"2016-02-02T09:50:59Z","updated_at":"2016-02-02T09:50:59Z","body":"Hi guys,\r\n\r\nI can run benchmarks on each separate improvement separately so we can see what makes a difference and what doesn't. I'm very behind schedule on a project at the moment, so I won't be able to do this for a few weeks.\r\n\r\nRegarding the sync.Pool usage & Close():\r\n\r\nI can make it grab the enc buffer from the sync.Pool on every write, which will be a negligible speed difference and it would technically improve the memory performance anyway in some applications. That would still be an improvement over the current version of snappy here. However, my inclusion of the buffer for small writes still requires the Close() because it pools small writes into one. For this purpose Close() is necessary as it is needed to write any remaining data stored in the buffer.\r\n\r\nI want to forward the following argument regarding the Close() according to the above description. I think this is necessary for the following reason:\r\n\r\nSnappy is currently worse than useless if it is used to write lots of small chunks instead of a few big chunks. I am using it to write binary data encoded from integers, so integers are encoded into slices of 1-8 bytes and these are written to the snappy writer. With this behavior snappy creates a new chunk with a chunk header of 8 bytes plus my 1-8 bytes, which of course is not compressed better than the uncompressed chunk, meaning that all I am doing is inflating the data by adding 8 bytes to every write, not compressing it. The solution for would be if I used bufio.Writer wrapped around the snappy.Writer. This is what you would all recommend as it makes perfect sense, except... do you really expect the users of this snappy package to either assume or check the code and know that no buffer is used? (Or even be smart enough to know they should use one?) Other compression methods do typically use a buffer to pool small writes (deflate does for example). Most users will simply assume it does do this already, or will not be experienced enough programmers to think of it, and the result is that they are inflating their data, not compressing it.\r\n\r\nThe truth of the matter is that the sync.Pool and the buffer (and therefore the Close function) are not necessary only if you assume the user knows how the snappy package works. The reality is that they don't, and while you can argue that that is their problem and they should become better programmers... they won't. So at the end of the day the question is whether you want the snappy package working efficiently out-of-the-box for everyone or whether you only want it working efficiently for programmers who already know what they're doing. It can be made to work out-of-the-box without any compromises on speed, but the one thing it needs for this is a Close() function.\r\n\r\nThe way I programmed the buffer on my snappy folk means that if the user is already implementing their own buffer then the bytes will be encoded directly, and if the user is not using a buffer then snappy's buffer will be used, and if the user doesn't need a buffer because they're writing a large slice of bytes then it will be written directly. It's foolproof.\r\n\r\nI think this point is worth discussing.\r\n\r\nThanks,\r\nAlasdair"}}