[PATCH] D93531: [clangd] Reuse buffer for JSONTransport::readRawMessage

sammccall added a comment. I've put a version of the reading part in D93653 . This seems a bit less invasive to me - I'm still not sure if it's worth the readability hit though... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D93531: [clangd] Reuse buffer for JSONTransport::readRawMessage

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks, this version seems just as clear as the original to me, and fewer allocations is nice :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D93531: [clangd] Reuse buffer for JSONTransport::readRawMessage

njames93 updated this revision to Diff 312930. njames93 added a comment. Remove input buffers, but keep output as its easy to reason about. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93531/new/ https://reviews.llvm.org/D93531 Files: clang-too

[PATCH] D93531: [clangd] Reuse buffer for JSONTransport::readRawMessage

sammccall added a comment. In D93531#2463126 , @njames93 wrote: > In D93531#2463052 , @sammccall wrote: > >> This adds a bit of complexity, making the code here a fair amount harder to >> follow and verify the corr

[PATCH] D93531: [clangd] Reuse buffer for JSONTransport::readRawMessage

njames93 updated this revision to Diff 312796. njames93 added a comment. Moved a few members out of the class. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93531/new/ https://reviews.llvm.org/D93531 Files: clang-tools-extra/clangd/JSONTransport

[PATCH] D93531: [clangd] Reuse buffer for JSONTransport::readRawMessage

njames93 added a comment. In D93531#2463052 , @sammccall wrote: > This adds a bit of complexity, making the code here a fair amount harder to > follow and verify the correctness of. > > - Do we have evidence that these allocations are causing a problem? (

[PATCH] D93531: [clangd] Reuse buffer for JSONTransport::readRawMessage

sammccall added a comment. This adds a bit of complexity, making the code here a fair amount harder to follow and verify the correctness of. - Do we have evidence that these allocations are causing a problem? (e.g. do we observe a significant decrease in RSS after the patch)? Naively I would ex

[PATCH] D93531: [clangd] Reuse buffer for JSONTransport::readRawMessage

njames93 updated this revision to Diff 312780. njames93 added a comment. Extend buffer behaviour to sendMessage. Calls to this function seem to be guarded(up the call stack) by a mutex so again we shouldn't worry about races. Besides there is no synchronization in the function currently when wri

[PATCH] D93531: [clangd] Reuse buffer for JSONTransport::readRawMessage

njames93 added a comment. 100% sure that failure is from trunk Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93531/new/ https://reviews.llvm.org/D93531 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D93531: [clangd] Reuse buffer for JSONTransport::readRawMessage

njames93 created this revision. njames93 added a reviewer: sammccall. Herald added subscribers: usaxena95, kadircet, arphaman. njames93 requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Reusing the buffers for reading m