[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.e
From: |
Theodor Thornhill |
Subject: |
bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el |
Date: |
Sun, 07 Mar 2021 21:16:18 +0100 |
Hi Dmitry,
> I just got around to testing this properly (sorry),
No worries
> and so far I've been
> able to reproduce the slow behavior only when there are many matches in
> the "big long line" file. I'm using a 500KB minified CSS as an example.
>
> When there are only a few matches, the search is relatively
> instantaneous. So that's a weird mismatch with your reports. If you have
> some details to add to reproduce the slowdown in the "few matches" case,
> that could be helpful too.
Hmm. Theres always a possibility of a human error on my part during the
benchmarks, of course!
>
> I'm currently looking at the patch and trying to figure out whether we
> could apply some smaller change, or a change not in xref--insert-xrefs
> (which is relatively complex already) with the same benefits.
>
Yeah, I also wanted to not add too much to that function, but I couldn't
get improvements other places :)
> Also:
>
> - Could you explain the change to xref--collect-matches-1 in the most
> recent patch? In my testing it doesn't move the needle at all, and it
> seems unnecessary because neither Grep or Ripgrep report matches on the
> same line separately with the current arguments that we pass to them.
> But if we did... what's the idea? Skip all subsequent matches, no matter
> if they're far or close?
>
Yeah, skipping subsequent matches yielded an improvement from ~1.09 to
~1.03 seconds, so not the biggest improvement, but it was consistent.
Thus I kept it.
> - What do you think about making an effort to actually retain all the
> matches in the output?
I see why we would want to do that, but as I mentioned in the last mail
I sent, these files are mostly "junk" anyways. However, it is probably
best to be able to retain them if we can. I just think speed should be
more important
> That would mean interpreting the xref-truncate-line-to value (or
> however the var could be renamed) as the maximum number of chars to
> render on the line *per match*. And if there is too much text between
> them, those parts can become "(truncated...)". Your current
> implementation can cut off valid matches, and we probably want to
> preserve them if feasible. OTOH, the default value could go down to
> 200 with this approach.
>
Yeah, I had an implementation where I "snipped" between matches and
concatenated them together, but that still yielded too large a line for
my emacs on a 3 million char length file, so I scrapped that idea. I
guess it still is possible, though!
> What I mean is, we can provide the "fullest featured" default behavior,
> one which never omits any valid matches and just truncated the line
> context around them, and the users who want even faster searches (at the
> cost of missing matches, esp. in find-replace scenarios) have something
> else to customize too.
Yeah, I think this is the best approach too. Especially for grep users.
I'll still probably use
(add-to-list
'xref-search-program-alist
'(ripgrep . "xargs -0 rg <C> -nH --no-messages -g '!*/' -e <R> -M 400
--max-columns-preview | sort -t: -k1,1 -k2n,2"))
Or something to that effect anyways :)
--
Theo
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, (continued)
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Theodor Thornhill, 2021/03/02
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Dmitry Gutov, 2021/03/02
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Theodor Thornhill, 2021/03/03
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Dmitry Gutov, 2021/03/03
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Theodor Thornhill, 2021/03/03
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Dmitry Gutov, 2021/03/06
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Dmitry Gutov, 2021/03/06
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Dmitry Gutov, 2021/03/06
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Theodor Thornhill, 2021/03/07
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Dmitry Gutov, 2021/03/07
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el,
Theodor Thornhill <=
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Theodor Thornhill, 2021/03/07
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Dmitry Gutov, 2021/03/07
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Dmitry Gutov, 2021/03/09
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Juri Linkov, 2021/03/03
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Gregory Heytings, 2021/03/03
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Dmitry Gutov, 2021/03/03
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Gregory Heytings, 2021/03/03
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Dmitry Gutov, 2021/03/03
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Gregory Heytings, 2021/03/03
- bug#46859: 28.0.50; [PATCH]: Add option to truncate long lines in xref.el, Gregory Heytings, 2021/03/03