-
-
Notifications
You must be signed in to change notification settings - Fork 642
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CIDER locks up when printing large strings #1115
Comments
Might be related to #228. A reliable repro case and some quality time with the debugger/profiler should solve this. |
@Malabarba I would agree that this is more than likely Emacs' various long lines issues more than CIDER on further review. |
I don't think the problem's just font-locking related, although it's likely a good idea to disable font-locking for bigger results. There's something pretty wrong happening in the REPL with respect to output - one bigger result/output and it becomes nearly unusable. And this has been happening before the introduction of "fancy" font-locking for the results and the output. |
I have previously managed to stop this by spamming C-g along with cider-interrupt, but it took a long time before it registered my desire for that process to just go off and die. |
@arrdem Btw, you do experience this problem only in the REPL or for interactive evaluation results as well? |
Confirm this |
Font-locking extremely long strings is pretty slow (and might generate errors in some cases), so it's now conditional. This behavior is adjustable via `cider-font-lock-max-length`.
Just noticed there's some fresh profiling data here. Seems our custom REPL implementation sucks big time... |
I don't think it's about REPL, it's the same emacs long-line problem, or somehow connected with it. REPL buffer inserts the result with no new lines. For example, if i do I'll try to test interrupting with big text but without long lines. |
I'm also running into this issue on a weekly-or-so basis. Very annoying. One thing to note is that it gets better (i.e. emacs stays slightly more responsive), when |
If the missing new lines is the problem, a simple solution is to use clojure.pprint/pprint to print out the result value. How do I change the (cider)repl to do this? |
Will in fact mostly mitigate this for many structures (nested maps etc) since the usual line length stays low enough that the REPL doesn't lock up. Happy belated birthday to this issue >.> I personally agree with @zarkone, this may well be Emacs' known problem with super long lines. Worth testing to see if this is worse than that, although the fact that it is possible to escape this hangup with |
Btw, do we have some profiling data regarding this? It'd be nice to know what should we aim to speed up/optimize... |
Even with toggle-truncate-lines and cider-repl-toggle-pretty-printing on, this problem has resurfaced with spec errors. There is no way to pretty print those, and if the :arg or :ret is a huge datastructure, emacs dies. |
@sooheon Could you give an example of such a spec error? |
@gganley Just create a spec requiring e.g. an int, and then pass it a gigantic clojure map. The entire map will be printed to show the erroneous value. |
Spending some time in the profiler with cider (master 18ed761) on Emacs 25.3. In Emacs, toggle off
In the repl, disable
Output a long lineStart the profiler. Generate a long line in the repl
this is the memory profile and CPU profile Notes:
Lag when long lines exist in the REPLNow that this long line exists in the REPL there is the familiar lag. Just typing "return" at the prompt gets you the I discovered I can interrupt this hang by typing any prefix that In the profiler that looks like this. I'm going to type return, wait a few seconds, type I'm not convinced |
But at the cost of now scrolling to the end of the long output. :-)
Hmm, seems you found some different bug (which should be simple to fix). The spinner gets triggered only while there's some evaluation in progress and just pressing return shouldn't trigger any evaluation. Looking at the profiler data I'm completely puzzled how ido gets involved in all of this. As for a fix I've been thinking the following: Results and output are streamed in chunks by nREPL (See https://github.com/clojure-emacs/cider/blob/master/cider-repl.el#L805). We can simply add a couple of running counters of the size of the result/output so far and when they reach some maximum we can offer users to truncate the long result (and maybe display the who result in a dedicated disposable buffer. We can also suggest users to remove the last output/result after they print it if it's something big. And last but not least - we can borrow unrepl's idea of expandable (elided) collections. It prints the beginning of a (big) collection and clickable ellipsis to show you a bigger part of it. We can store in memory big results off-screen and attach some simple relation between a short version with ellipsis and the actual result. I think all of the above a relatively simple to implement and would be very appreciated by our users. |
|
Well, for some reason it is triggering an evaluation (of the newline?), see here
I'll go track that down. |
So here's what I'm finding about the lag when long lines exist in the REPL. If I watch the |
Hmm, that's extremely odd. Might be somehow related to the fact the REPL buffer is also the connection buffer, but I cannot think why the processing of the responses would halt and continue when you type something. |
Somehow related to the spinner itself. I just tried |
Hummm I was able to repro this behavior using just |
The code you wrote to insert |
I don't see how this would be a problem. The evaluation would still be complete, we'd just return only a portion of the result and some marker to be able to retrieve the full result (e.g. when an user presses |
FWIW this is a well known problem with emacs redisplay when long lines are present. It occurs in all buffers and it's way worse with truncate-lines non-nil. Just copy repl output into a fundamental buffer and you should observe slow edditing. The only solution (in my mind) is to truncate long lines in the process filter. Spinner is not the root cause. It just triggers redisplay 10 times a seconds. Setting fps to 1 removes the problem. As a replacement for the spinner making the last REPL prompt red would work for me, but please don't leave us without eval indicator. |
It's a big advantage on remotes. One problem with the server side is that one would need to manage caching. Storing all output's so far doesn't make sense. Also I am not sure this should apply to |
This should definitely not apply for
I was thinking of storing just results/output that exceed some limit, but we can probably limit this just to the last result. The problem with not storing everything (either client or server-side) is that you won't be able to expand all the truncated results/output. |
I would bet people want access to the last N results, where N might not be "every result for all time" but it seems likely N is also greater than 1 for most people. It suggests to me that the cache eviction should be directed by the client. Then the client is free to implement a "sliding window" policy with configurable N, keep just the last result (N=1), allow specific results to be "pinned" regardless where they are in the history, disable caching entirely (N=0) if it doesn't want to be bothered, etc. |
fwiw I am working on this. I have a prototype middleware intercepting responses containing "value" or "pprint-out" and truncating those strings if their length exceeds some threshold. The middleware keeps a hash map of the full-length strings which can be retrieved or removed via middleware ops. I'm moving now into cider to change the nrepl response handlers. Those will need to add a button whenever the response has been truncated, such that clicking the button will retrieve the full length result and display it (in a separate window, I'm not going to attempt folding). Finally I'll see to it that clearing these truncated lines of output from the repl will also clear the corresponding data in the middleware. In principle this should prevent excessive network traffic from remote repls, as well as eliminate long lines in the repl buffer leading to other interactivity problems. The end goal is to make sure the middleware is not cider-specific so other tools can (if they choose) enable the truncation too. |
Sounds awesome! 👍
You should also consider Just one small consideration - there should be some op to turn the interception on and off. I assume everyone will want it, but early version might have some bugs and it'd be nice if users can work around those without having to meddle with the middleware vector. |
Is it about truncating the length of individual lines or the whole output? If the latter, would be nice to elide the middle part of the output, keeping start and end for inspection. Especially useful for commands like Thanks for working on this! |
@bbatsov - the middleware is disabled if the threshold length is nil. @vspinu I agree, those would be useful. It's currently truncating the entire result, and by that I mean it blindly chops the result string at the specified length. There's no other smarts involved: no logic to elide elements of the collection in some logical way, no attempt to fold nested data structures, not even trying to preserve the closing braces. I'm going to finish a pass with this simplistic scheme and make sure the mechanics are sound. I might have to save any kind of sophisticated "elision" for a subsequent release. |
@gonewest818 something to consider when not preserving structure is that anyone with smartparens or lispy or the like enabled will likely face a ton of lag as the plugin tries to search for matching pairs. |
Good point.
… On Mar 26, 2018, at 1:49 AM, Sooheon Kim ***@***.***> wrote:
@gonewest818 something to consider when not preserving structure is that anyone with smartparens or lispy or the like enabled will likely face a ton of lag as the plugin tries to search for matching pairs.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Just saw this https://twitter.com/CursiveIDE/status/1023508436443529216 and got reminded of this ticket. @gonewest818 If you don't have time to wrap this up maybe you can at least push your code to some branch so someone else can take it over and the work won't be lost. That's definitely not a pressing concern, as we have our hands full with other tasks, but I think it'd be great if something like this was handled directly in nREPL itself. And now that's finally feasible. :-) |
Just one comment from my side. I am doing some type of NLP with clojure, so my data structures are by definition very long. But each time I have a syntax error in my repl expression (missing parenthesis for example), This is very annoying. I think we really need a,configurable, absolute limit on the length of "result" of a given repl expression cider is willing to print. (if "error", or "out", this should not matter) For me it would be acceptable if it just "cuts" the long "result" in the middle, without paying any attention to structure. |
To illustrate see this: The following snippet has unmatched parenthesis:
Trying to evaluate this in emacs it fails obviously, and
which results in a hanging emacs immediately. |
I have found an other potential work around to the problem of large strings- In my work with clojure for analyzing (text) data, I noticed that I hardly ever want to print large strings in full. So one way of solving the large string problem, is to think about using by default a pretty printer, I changed for this purpose the implementation of fipp, so that it truncates all strings to 10 characters and then prints the string length in brackets. This has a a side effect that most nested maps / text structures are very readable in repl, like this:
Combining this with using print-length and print-level nearly guaranties that the the output stays reasonable short. I was even thinking that clojure.core should get a var like print-string-length, In case I really want to see a string in full length, I could then use (print ) or (pprint ) directly. Ideally the fipp library would have a at runtime configurable way to control the string truncation. Any opinion on this ? |
Another, rather easy solution, to the large string problem is to set a specific 'print-method' for strings. The following code does this: (defn shorten-string [s]
(if (> (count s) 100)
(str (.substring s 0 100) " ...[" (count s) "]")
s))
(defmethod print-method java.lang.String [v ^java.io.Writer w] (.write w (shorten-string v)))
I combine this typically with a method to truncate the overall length of the result of a REPL evaluation to 1000, see my comment on #1934 Both methods together give a flexible way to safely explore even larger data structures in clojure / cider. |
Just to give my conclusion on this problem. I experimented a lot in solving the long-string problem, and I came to the conclusion that by combining to set *print-length* , *print-level* ,the usage of cider pretty-printing and customizing the print-method function of java.lang.String via
the problem of too long cider-repl outputs (including very long Strings) can be nearly eliminated completely. It is rather convenient already now in the repl to change quickly the settings of
To have a convenient way to control the string truncation, two simple elisp function can be used, which inject into the repl the clojure code to control the printing of java.lang.String: (defun cider-pr-shorten-string (max)
(interactive "nShorten strings to ?: ")
(insert (format "
(defn shorten-string [s]
(if (> (count s) %d)
(str (.substring s 0 %d) \" ...[\" (count s) \"]\")
s))
(defmethod print-method java.lang.String [v ^java.io.Writer w] (.write w (shorten-string v)))"
max max) )
(cider-repl-return))
(defun cider-pr-normal-string ()
(interactive)
(insert "
(defmethod print-method java.lang.String [v ^java.io.Writer w] (.write w v))
")
(cider-repl-return))
This should nearly guaranty that the evaluation of clojure expressions never returns too long output. The "shorten" function prints the length of the string at the end, so it is rather easy to see, how long the string would have been, if printed. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding! |
That's mostly solved on CIDER 0.21+, so I'll close this ticket. |
UPDATE: cider version 0.22.1-snapshot and Emacs 26.3; Mac OS X 10.14.6; JDK 11 If I try to print a long string a couple of times the REPL still does get significantly slower.
|
I have of late sacrificed many Emacsen to print lockup. Either due to nREPL network transport times or Emacs' known performance issues with long lines it's possible for a CIDER repl to freeze Emacs entirely such that it's not even possible to
(kbd C-c C-c)
or(kbd C-c C-b)
to abort the print in progress. Killing Emacs is (obviously) a huge cramp on my workflow and it'd be awesome if this didn't happen.<3
The text was updated successfully, but these errors were encountered: