Use lower nREPL print quotas for interactive evaluation

[vemv]

Fixes #3633

Seems to work nicely (per the nrepl logs, and evaluating some huge objects).

Marking the PR as draft as I'll want to test out the defuns one by one.

Although many are affected, a good variety of defuns are unchanged - we still use the larger quota for repls, stacktraces, logs, tests, etc - basically whenever the nREPL output is printed to a buffer (vs. to an overlay).

I based the quota size on the frame, as definitionally one cannot display an overlay larger than the current frame.

Cheers - V

[bbatsov]

Let me think about this one for a while. I get your point, but I'm wondering if that's the right solution to the problem - this quota exists mostly to avoid out of memory errors on the nREPL server side, for limiting printing there might be other options we can consider.

[vemv]

this quota exists mostly to avoid out of memory errors on the nREPL server sid

TIL tbh!

I quite often see the limit trimming the REPL output, so I thought that was big part of its reason to exist. Or at least it's a fortunate side-effect.

[bbatsov]

Maybe you should use the naming print and pprint in both helpers, as I find the current names confusing.

[bbatsov]

And change cider--nrepl-pr-request-map's name to match.

[cichli]

The pr one is so-named because it always uses clojure.core/pr, regardless of your printing options. The print one uses whatever your cider-print-fn is set to (which might also be pr, and so will not necessarily pretty print).

[bbatsov]

I'm struggling to understand the logic of those calculations.

[alexander-yakushev]

Seems pretty discernable to me – the limit is 1.5 full frames of text (one full frame and then some extra on the side). This can be said in a comment nearby.

[bbatsov]

I couple of general concerns?

• How will the trimmed results look exactly? (both in the minibuffer and in the overlay) I believe we use the same handler for both, so at the very least it should be clear that something got trimmed and what caused this to happen.
• By trimming results at the nREPL level it won't be possible for people to request the full result to be rendered
• I'm not sure if the interactive results are streamed (I think not, even if we discussed it with @yuhan0 in the past, if I remember correctly), so perhaps we can make the streamable and if the first chunk is full we can just assume something is relatively big and display it in a buffer instead of an overlay.

I'm also wondering how big of a problem is this in practice as people have rarely complained about it.

[bbatsov]

I'd probably name this cider--calculate-interactive-print-quota.

[bbatsov]

Ping :-)

[cichli]

FWIW: when I added the default quota we didn't even have the evaluation result overlays! I think a smaller quota definitely makes sense there (and perhaps anywhere we insert into the current buffer) but the larger one still makes sense for cider--pprint-eval-form since it outputs to a popup buffer. In fact if I know an eval output will be large I prefer to use a popup buffer over the REPL. I realise that breaks the "interactive eval" distinction though.

this quota exists mostly to avoid out of memory errors on the nREPL server side

Streamed printing solved that (for well-behaved pretty printers like fipp) but adding the quota was also to prevent the server overwhelming the client – it used to be easy to cause emacs to lock up by accidentally evaluating a huge data structure e.g. CLJS compiler state or an infinite seq.

[alexander-yakushev]

Strictly speaking, this one is not merely a "representational" printing since the result will substitute the code that was evaluated in the buffer. If we truncate that one, user will get incorrect code inserted into their source file (and it will break Paredit too 🤢).

It's not apparent what would be the best UX. Perhaps, doing no replacement and showing the message that the evaluation result is too big to be inserted into the code; but inserting a truncated result is probably not on that list.

[vemv]

Good catch, yes, as mentioned in OP these defuns deserve being reviewed one by one.

Perhaps, it's doing no replacement and showing the message that the evaluation result is too big to be inserted into the code

If it's > 1MB (our default quota as of master) that would seem reasonable to me. Maybe preview its first lines and ask for a y/n confirmation.

Either way, this is good to simply exclude from the PR for now.

[alexander-yakushev]

Regarding this and the preceding one: REPL is usually a bit more sturdy to large strings being printed, and the REPL can be scrolled, so a limit of 1 or 1.5 frame is not as clear cut for the REPL. It's true that Emacs still doesn't like huge one-liners and would lag barely/somewhat/terribly on it; but users might still expect that the REPL would handle a large value, and that they can C-a to the beginning and copy the vlaue to kill ring, for example.

That still wouldn't work for values over 1MB as we already truncate those in the REPL too, but between 10k and 1M there is a big area that somebody can be accustomed to.

[vemv]

Sounding reasonable.

I distinctly remember my repl getting slower before the 1MB limit saving the day. Even then, it was an unpleasant experience and has wished the limit came in earlier.

Perhaps we can have a slightly more generous limit e.g. 5 'screenfuls'.

...There will always being some tension between a responsive repl, and being opinionated.

I prefer to be opinionated (i.e. nudge users to use spit if they want a huge result to be grabbable) so that we can reasonably guarantee that the repl is generally always usable.

[alexander-yakushev]

Kinda shows that SLIME with its clickable buttons in the REPL did one thing right 🤔.

[alexander-yakushev]

I distinctly remember my repl getting slower before the 1MB limit saving the day. Even then, it was an unpleasant experience and has wished the limit came in earlier.

I tried printing values into the REPL that go over 1MB limit, and while they make Emacs lag slightly when moving the cursor across the REPL, there wasn't anything like Emacs-freezing-100%-cpu-disaster that I remember happening a long time ago. Can you reproduce something like that on your setup? That can help pick up the quota limit better.

[bbatsov]

5 months later - we should probably decide on how to proceed with this PR. :-)

[vemv]

This still seems to me a good improvement, but merging it would imply maintaining it which I cannot readily do nowadays.

Let's close - we can re-open later.