search

eclipse-lsp4j / lsp4j

A Java implementation of the language server protocol intended to be consumed by tools and language servers implemented in Java.
https://eclipse.org/lsp4j
Other
581 stars 141 forks source link

Avoid temporary String object creation in StreamMessageProducer #816

Closed sebthom closed 4 months ago

sebthom commented 4 months ago

This can reduce the likeliness of OOMs when dealing with large response messages from LS servers. See discussion at https://github.com/eclipse-lsp4j/lsp4j/issues/815

sebthom commented 4 months ago

No, it won't solve the issue but it allows for processing of larger messages without getting an OOM.

jonahgraham commented 4 months ago

I'll leave it open for now for others to have a look. But assuming no dissenting views it can be merged soonish.

Thanks to @pisv we have a positive review - even better than simply no dissenting views! Waiting to hear from @sebthom if this commit or #817 should be submitted first (as I assume some amount of merging conflict resolution may be required)

sebthom commented 4 months ago

I'll leave it open for now for others to have a look. But assuming no dissenting views it can be merged soonish.

Thanks to @pisv we have a positive review - even better than simply no dissenting views! Waiting to hear from @sebthom if this commit or #817 should be submitted first (as I assume some amount of merging conflict resolution may be required)

Rebasing this PR is easier, so I would prefer if #817 gets merged first.

jonahgraham commented 4 months ago

Thanks @sebthom - I'll hold off until @pisv reviews #817

@pisv feel free to submit #817 if you approve (and #816 once it is rebased)