GNU bug report logs - #50806
27.2; [PATCH] Optimize ansi-color.el

Previous Next

Full log

Message #13 received at 50806 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: miha <at> kamnitnik.top, 50806 <at> debbugs.gnu.org
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: Re: bug#50806: 27.2; [PATCH] Optimize ansi-color.el
Date: Sun, 3 Oct 2021 10:53:20 -0700

(Apologies for the delay in replying; the original message hit my spam 
filter, and I only saw the followup. CC'ing Lars, who reviewed my 
patches and can hopefully provide a more-helpful review than me with my 
shaky Lisp skills.)

On 9/25/2021 3:32 PM, miha--- via Bug reports for GNU Emacs, the Swiss 
army knife of text editors wrote:
> Attached patch speeds up ansi-color.  It tries to eliminate as many
> allocations (cons and list) as possible.
> 
> Benchmarks, recorded in emacs -q, are in the second attachments.  With
> the patch applied, there is a 26% speedup in elapsed time, mostly
> because the garbage collector has to take care of less allocations.

Sounds good to me. I did some very light benchmarking on my patches, but 
only to make sure I didn't make things too much slower (hopefully I 
succeeded). Making it faster would definitely be nice.

> Two less important side notes:
> 
> 1) This patch additionally makes it very straight forward to add support
>     for ANSI color codes 38 and 48 allowing 256-colors and 24bit
>     full-color.  I plan to submit such a patch later (for both ansi-color
>     and term-mode).

Glad to hear it! I was planning on doing this for Emacs 29, but if 
you've already got something in mind for this, feel free to work on it. 
Another thing that might be worth looking at would be improving 
term.el's support for "basic" ANSI escapes; I think it only does bold, 
underline, and inverse. Also, the inverse implementation seems a bit 
over-complicated when it could just use the :inverse-video face 
attribute instead.

> 2) Two vector variables, that were recently added by Jim, were merged
>     into one.  Adding him as CC.

I see you posted an updated patch that doesn't merge these vectors. I 
don't have an opinion here, although if we do merge them, it would 
probably be nice to get that into Emacs 28; other packages might 
conceivably want to let-bind those[1].

[1] See man.el, which let-binds `ansi-color-basic-faces-vector' in 
`Man-fontify-manpage'. That's for the basic faces, not the colors, but 
the same idea should apply.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.