Slow in some pathological cases #19
Comments
|
I'd also note that |
purcell
added a commit
that referenced
this issue
Nov 3, 2019
|
For now, I have pushed the simple fix, which is to avoid |
|
Hello @purcell, I did a quick benchmark when formatting OCaml code with I wanted OCamlFormat to use In practice, I never had any issue whether |
|
Closing, since there is no known speed issue currently. |
jimeh
added a commit
to jimeh/reformatter.el
that referenced
this issue
May 23, 2020
Borrowed from go-mode's gofmt and releated functions: https://github.com/dominikh/go-mode.el/blob/734d5232455ffde088021ea5908849ac570e890f/go-mode.el#L1850-L1956 Hence I'm not the original author of most this code, but I have used a number of borrowed variations of it over the years for hand-rolled formatting solutions. It works by passing the current buffer content to `diff` via STDIN to compare it against the target file on disk producing a RCS formatted diff that easy to parse. It then iterates through the said diff only modifying relevant lines to make the buffer be identical to the target file. In my experience it does a much better job of maintaining cursor position compared to `insert-file-contents`. Performance is also excellent and near instant. On a late-2016 13-inch MacBook Pro with a Dual-Core i7 at 3.3Ghz, I can't really tell the difference between this solution and `insert-file-contents`. I did also test it against the Haskell example from: purcell#19 Replacing buffer content of `Req.hs` with that of `Req-reformatted.hs` is most of the time just as fast as `insert-file-contents` (around 50ms), and only sometimes a little slower of around 100-150ms based on my totally unscientific measurement technique of guesstimating it. I did also test the old `replace-buffer-contents` based variant too with `Req.hs`, and that took around 15 seconds.
jimeh
added a commit
to jimeh/reformatter.el
that referenced
this issue
May 23, 2020
Borrowed from go-mode's gofmt and releated functions: https://github.com/dominikh/go-mode.el/blob/734d5232455ffde088021ea5908849ac570e890f/go-mode.el#L1850-L1956 Hence I'm not the original author of most this code, but I have used a number of borrowed variations of it over the years for hand-rolled formatting solutions. It works by passing the current buffer content to `diff` via STDIN to compare it against the target file on disk producing a RCS formatted diff that is easy to parse. It then iterates through the said diff only modifying relevant lines to make the buffer be identical to the target file. In my experience it does a much better job of maintaining cursor position compared to `insert-file-contents`, as it only modifies lines that got corrected, and leaves all other lines in the buffer untouched. Performance is also excellent and near instant. On my late-2016 13-inch MacBook Pro with a Dual-Core i7 at 3.3Ghz, I can't really tell the difference between this solution and `insert-file-contents`. I did also test it against the Haskell example from: purcell#19 Replacing buffer content of `Req.hs` with that of `Req-reformatted.hs` is most of the time just as fast as `insert-file-contents` (around 50ms), and only sometimes a little slower of around 100-150ms based on my totally unscientific measurement technique of guesstimating it. I find that rather impressive as the RCS-formatted diff itself is 17KB. I also tested the old `replace-buffer-contents`-based variant to see how it perform on my machine. It took around 15 seconds.
jimeh
added a commit
to jimeh/reformatter.el
that referenced
this issue
May 23, 2020
Borrowed from go-mode's gofmt and releated functions: https://github.com/dominikh/go-mode.el/blob/734d5232455ffde088021ea5908849ac570e890f/go-mode.el#L1850-L1956 Hence I'm not the original author of most this code, but I have used a number of borrowed variations of it over the years for hand-rolled formatting solutions. It works by passing the current buffer content to `diff` via STDIN to compare it against the target file on disk producing a RCS formatted diff that is easy to parse. It then iterates through the said diff only modifying relevant lines to make the buffer be identical to the target file. In my experience it does a much better job of maintaining cursor position compared to `insert-file-contents`, as it only modifies lines that got corrected, and leaves all other lines in the buffer untouched. Performance is also excellent and near instant. On my late-2016 13-inch MacBook Pro with a Dual-Core i7 at 3.3Ghz, I can't really tell the difference between this solution and `insert-file-contents`. I did also test it against the Haskell example from: purcell#19 Replacing buffer content of `Req.hs` with that of `Req-reformatted.hs` is most of the time just as fast as `insert-file-contents` (around 50ms), and only sometimes a little slower of around 100-150ms based on my totally unscientific measurement technique of guesstimating it. I find that rather impressive as the RCS-formatted diff itself is 17KB. I also tested the old `replace-buffer-contents`-based variant to see how it performs on my machine. It took around 15 seconds.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In the case of reformatting this Haskell file with
ormoluv0.0.1.0, replacing the buffer contents takes an extraordinary and unacceptable amount of time (from 30s to many minutes), even though the formatter program itself executes quickly (< 0.25s).Inputs/outputs are captured in this gist. They are somewhat pathological, in that the input source file is 64kb and the resulting diff is 36kb with 202 modified regions.
The problem is visible in Emacs 26.3: in Emacs > 26.1 we use Emacs'
replace-buffer-contentsfunction, which aims to preserve text properties, regions, markers etc. as much as possible during operations such as this. (The closest alternativeinsert-file-contentsmakes only limited attempts to do this, but generally preservespointquite effectively.)It turns out that this is a known issue with
replace-buffer-contents, which has even subsequently gained a new argument to limit execution time, after which it will revert to a simpler buffer content replacement method. This seems like a strange design choice.The options for dealing with this in
reformatter.elare, roughly:replace-buffer-contentsentirely. Most reformatting code "in the wild" does this anyway, of course, because it pre-datesreplace-buffer-contents.replace-buffer-contentsonly if it supports the time limit argument, and set the time limit to something like 0.5s. This makes the results somewhat nondeterministic, but may generally give the best results for users in the usual fast-replacement case. Here, also, the fallback behaviour may not work as well as justinsert-file-contents, so this would require a little testing.replace-buffer-contentsonly if the current file is sufficiently small, ie. unlikely to be pathological. This limit might be around 5kb.I'd probably favour the first approach (ie. avoid
replace-buffer-contents)./cc @mrkkrp, who raised this issue, and @wbolster & @ludwigpacifici, who encouraged the use of
replace-buffer-contentsin #10.The text was updated successfully, but these errors were encountered: