* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
@ 2020-05-12 4:30 Clément Pit-Claudel
2020-05-12 6:42 ` martin rudalics
` (3 more replies)
0 siblings, 4 replies; 57+ messages in thread
From: Clément Pit-Claudel @ 2020-05-12 4:30 UTC (permalink / raw)
To: 41200
[-- Attachment #1: Type: text/plain, Size: 11315 bytes --]
Hi all,
I've recently noticed that opening a tooltip on my machine takes about 0.5s when x-gtk-use-system-tooltips is set to nil.
I bisected my config, and… nothing. It's not one package: instead, it's an accumulation of small slowdowns.
Is seems that defining a face makes x-show-tip a tiny bit slower, but these effects stack.
Here is a repro:
(defun my-def-many-faces (nfaces)
(dotimes (i nfaces)
(custom-declare-face
(intern (format "my-face-%d" i))
'((t)) "A face."
:group 'basic-faces)))
(defun my-bench-x-tip (nfaces)
(setq x-gtk-use-system-tooltips nil)
(my-def-many-faces nfaces)
(benchmark-run 1 (x-show-tip "Test" (selected-frame) nil 5 nil nil)))
(my-bench-x-tip 100) ;; ⇒ (0.035934318 1 0.015908304000000012)
(my-bench-x-tip 200) ;; ⇒ (0.049593474 1 0.01508625500000002)
(my-bench-x-tip 300) ;; ⇒ (0.094929297 2 0.03376510099999999)
(my-bench-x-tip 400) ;; ⇒ (0.094900665 2 0.03254889999999999)
(my-bench-x-tip 500) ;; ⇒ (0.118183442 2 0.03218763600000002)
(my-bench-x-tip 600) ;; ⇒ (0.154759438 3 0.04923829399999996)
(my-bench-x-tip 700) ;; ⇒ (0.183241646 3 0.04901039700000004)
(my-bench-x-tip 800) ;; ⇒ (0.212218872 3 0.050182316999999976)
(my-bench-x-tip 900) ;; ⇒ (0.248743542 3 0.04915146899999995)
(my-bench-x-tip 1000) ;; ⇒ (0.29221963 3 0.04943874300000006)
(my-bench-x-tip 1100) ;; ⇒ (0.334084605 3 0.05403986499999991)
(my-bench-x-tip 1200) ;; ⇒ (0.397292289 4 0.06869684599999992)
(my-bench-x-tip 1300) ;; ⇒ (0.442873256 4 0.06865671799999995)
(my-bench-x-tip 1400) ;; ⇒ (0.492474982 4 0.06888139900000001)
(my-bench-x-tip 1500) ;; ⇒ (0.579180262 5 0.08583425400000011)
(my-bench-x-tip 1600) ;; ⇒ (0.63504114 5 0.08973981699999989)
(my-bench-x-tip 1700) ;; ⇒ (0.723722857 5 0.09094433899999999)
(my-bench-x-tip 1800) ;; ⇒ (0.791952279 5 0.08777533800000015)
(my-bench-x-tip 1900) ;; ⇒ (0.902377982 6 0.10768666300000018)
(my-bench-x-tip 2000) ;; ⇒ (0.998815784 6 0.11384837999999986)
Be sure to run it in emacs -q, not emacs -Q, because emacs -Q ignores X resources and hence skips the body of make-face-x-resource-internal, which contributes greatly to the issue.
For some reasons the effects are a bit worse in my config — roughly a factor 3 to 5 (I have 600 faces defined, and each tooltip takes .5s to display). The profiles below suggest that face-spec-set-2 is called in my config, but not in my repro, which could explain part of the difference.
This is what the profile in emacs -q looks like:
- command-execute 1742 97%
- call-interactively 1742 97%
- funcall-interactively 1720 96%
- eval-defun 1711 95%
- elisp--eval-defun 1711 95%
- eval-region 1711 95%
- let 1711 95%
- list 1711 95%
- let 1711 95%
- x-show-tip 1708 95%
- face-set-after-frame-default 1708 95%
- face-spec-recalc 1654 92%
- make-face-x-resource-internal 1414 78%
- set-face-attributes-from-resources 1413 78%
- set-face-attribute-from-resource 1394 77%
- face-name 1353 75%
- check-face 1348 75%
facep 1344 75%
- face-spec-reset-face 239 13%
- apply 239 13%
set-face-attribute 234 13%
And this is what it looks like in my config:
- command-execute 1423 87%
- call-interactively 1423 87%
- apply 1423 87%
- call-interactively@ido-cr+-record-current-command 1423 87%
- apply 1423 87%
- #<subr call-interactively> 1423 87%
- funcall-interactively 1423 87%
- eval-defun 1345 83%
- apply 1345 83%
- #<compiled 0x1fa5d1dc39debc9e> 1345 83%
- elisp--eval-defun 1345 83%
- eval-region 1344 83%
- apply 1344 83%
- #<lambda -0x120930d847119138> 1344 83%
- endless/eval-overlay 1344 83%
- apply 1343 83%
- #<subr eval-region> 1343 83%
- my-bench-x-tip 1343 83%
- let 1280 79%
- list 1280 79%
- let 1280 79%
- x-show-tip 1277 78%
- face-set-after-frame-default 1277 78%
- face-spec-recalc 1218 75%
- face-spec-set-2 673 41%
- apply 672 41%
- set-face-attribute 671 41%
- internal-set-lisp-face-attribute 669 41%
- frame-set-background-mode 651 40%
- face-spec-recalc 411 25%
- make-face-x-resource-internal 352 21%
- set-face-attributes-from-resources 350 21%
- set-face-attribute-from-resource 343 21%
- face-name 312 19%
- check-face 309 19%
facep 308 19%
+ face-spec-reset-face 56 3%
face-spec-choose 1 0%
+ face-spec-set-2 1 0%
- face-attr-match-p 235 14%
face-attribute 235 14%
- make-face-x-resource-internal 321 19%
- set-face-attributes-from-resources 320 19%
- set-face-attribute-from-resource 316 19%
- face-name 296 18%
- check-face 294 18%
facep 293 18%
- face-spec-reset-face 223 13%
- apply 223 13%
set-face-attribute 219 13%
+ my-def-many-faces 63 3%
+ cider--make-result-overlay 1 0%
+ end-of-defun 1 0%
+ smex 78 4%
+ ... 188 11%
+ timer-event-handler 4 0%
+ redisplay_internal (C function) 2 0%
+ flyspell-post-command-hook 1 0%
I've attached both profiles.
Clément.
Configured using:
'configure -C'
Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF
ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS LIBSYSTEMD
JSON PDUMPER LCMS2 GMP
Important settings:
value of $LC_MONETARY: en_DK.UTF-8
value of $LC_NUMERIC: en_DK.UTF-8
value of $LC_TIME: en_DK.UTF-8
value of $LANG: en_US.UTF-8
value of $XMODIFIERS: @im=ibus
locale-coding-system: utf-8-unix
Major mode: Fundamental
Minor modes in effect:
tooltip-mode: t
global-eldoc-mode: t
electric-indent-mode: t
mouse-wheel-mode: t
tool-bar-mode: t
menu-bar-mode: t
file-name-shadow-mode: t
global-font-lock-mode: t
blink-cursor-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
buffer-read-only: t
line-number-mode: t
transient-mark-mode: t
Load-path shadows:
None found.
Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame minibuffer cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
cairo move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)
Memory information:
((conses 16 48162 5363)
(symbols 48 6344 1)
(strings 32 15896 1092)
(string-bytes 1 517314)
(vectors 16 10213)
(vector-slots 8 140571 9444)
(floats 8 19 41)
(intervals 56 230 7)
(buffers 992 11))
[-- Attachment #2: tip.emacs-q.prof --]
[-- Type: text/plain, Size: 3592 bytes --]
[profiler-profile "24.3" cpu #s(hash-table size 65 test equal rehash-size 1.5 rehash-threshold 0.8125 data ([redisplay sit-for execute-extended-command funcall-interactively call-interactively command-execute nil nil nil nil nil nil nil nil nil nil] 4 [sit-for execute-extended-command funcall-interactively call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil] 3 [let list let eval-region elisp--eval-defun eval-defun funcall-interactively call-interactively command-execute nil nil nil nil nil nil nil] 3 [facep check-face face-name set-face-attribute-from-resource set-face-attributes-from-resources make-face-x-resource-internal face-spec-recalc face-set-after-frame-default x-show-tip let list let eval-region elisp--eval-defun eval-defun funcall-interactively] 1344 [set-face-attribute-from-resource set-face-attributes-from-resources make-face-x-resource-internal face-spec-recalc face-set-after-frame-default x-show-tip let list let eval-region elisp--eval-defun eval-defun funcall-interactively call-interactively command-execute nil] 41 [face-name set-face-attribute-from-resource set-face-attributes-from-resources make-face-x-resource-internal face-spec-recalc face-set-after-frame-default x-show-tip let list let eval-region elisp--eval-defun eval-defun funcall-interactively call-interactively command-execute] 5 [set-face-attributes-from-resources make-face-x-resource-internal face-spec-recalc face-set-after-frame-default x-show-tip let list let eval-region elisp--eval-defun eval-defun funcall-interactively call-interactively command-execute nil nil] 19 [face-set-after-frame-default x-show-tip let list let eval-region elisp--eval-defun eval-defun funcall-interactively call-interactively command-execute nil nil nil nil nil] 54 [set-face-attribute apply face-spec-reset-face face-spec-recalc face-set-after-frame-default x-show-tip let list let eval-region elisp--eval-defun eval-defun funcall-interactively call-interactively command-execute nil] 234 [face-spec-recalc face-set-after-frame-default x-show-tip let list let eval-region elisp--eval-defun eval-defun funcall-interactively call-interactively command-execute nil nil nil nil] 1 [check-face face-name set-face-attribute-from-resource set-face-attributes-from-resources make-face-x-resource-internal face-spec-recalc face-set-after-frame-default x-show-tip let list let eval-region elisp--eval-defun eval-defun funcall-interactively call-interactively] 4 [make-face-x-resource-internal face-spec-recalc face-set-after-frame-default x-show-tip let list let eval-region elisp--eval-defun eval-defun funcall-interactively call-interactively command-execute nil nil nil] 1 [apply face-spec-reset-face face-spec-recalc face-set-after-frame-default x-show-tip let list let eval-region elisp--eval-defun eval-defun funcall-interactively call-interactively command-execute nil nil] 5 [nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 2 [tooltip-hide nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [completing-read-default completing-read read-extended-command byte-code call-interactively command-execute nil nil nil nil nil nil nil nil nil nil] 13 [read-from-minibuffer completing-read-default completing-read read-extended-command byte-code call-interactively command-execute nil nil nil nil nil nil nil nil nil] 9 [profiler-report funcall-interactively call-interactively command-execute execute-extended-command funcall-interactively call-interactively command-execute nil nil nil nil nil nil nil nil] 2 [Automatic\ GC] 47)) (24250 7797 63752 520000) nil]
[-- Attachment #3: tip.personal-config.prof --]
[-- Type: text/plain, Size: 37568 bytes --]
[profiler-profile "24.3" cpu #s(hash-table size 65 test equal rehash-size 1.5 rehash-threshold 0.8125 data ([set-face-attribute page-break-lines--update-display-table mapc page-break-lines--update-display-tables redisplay_internal\ \(C\ function\) nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 140 [ispell-send-string flyspell-word flyspell-post-command-hook nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [apply eldoc-minibuffer-message eldoc-message eldoc-print-current-symbol-info "#<compiled 0x16f6975adace3adf>" apply timer-event-handler nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 2 [syntax-ppss beginning-of-defun-raw end-of-defun elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [elisp--beginning-of-sexp elisp--fnsym-in-current-sexp elisp-eldoc-documentation-function run-hook-with-args-until-success eldoc-documentation-default eldoc-print-current-symbol-info "#<compiled 0x16f6975adace3adf>" apply timer-event-handler nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [intern custom-declare-face while let my-def-many-faces my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [make-face make-empty-face face-spec-set custom-declare-face while let my-def-many-faces my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 18 [set-face-attribute-from-resource set-face-attributes-from-resources make-face-x-resource-internal face-spec-recalc face-spec-set custom-declare-face while let my-def-many-faces my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 5 [custom-handle-keyword custom-handle-all-keywords custom-declare-face while let my-def-many-faces my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 13 [facep make-face make-empty-face face-spec-set custom-declare-face while let my-def-many-faces my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 15 [set-face-attributes-from-resources make-face-x-resource-internal make-face make-empty-face face-spec-set custom-declare-face while let my-def-many-faces my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [face-name set-face-attribute-from-resource set-face-attributes-from-resources make-face-x-resource-internal make-face make-empty-face face-spec-set custom-declare-face while let my-def-many-faces my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [apply face-spec-reset-face face-spec-recalc face-spec-set custom-declare-face while let my-def-many-faces my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [facep check-face face-name set-face-attribute-from-resource set-face-attributes-from-resources make-face-x-resource-internal make-face make-empty-face face-spec-set custom-declare-face while let my-def-many-faces my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [make-face-x-resource-internal make-face make-empty-face face-spec-set custom-declare-face while let my-def-many-faces my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 2 [set-face-attribute-from-resource set-face-attributes-from-resources make-face-x-resource-internal make-face make-empty-face face-spec-set custom-declare-face while let my-def-many-faces my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 3 [check-face face-name set-face-attribute-from-resource set-face-attributes-from-resources make-face-x-resource-internal make-face make-empty-face face-spec-set custom-declare-face while let my-def-many-faces my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [facemenu-add-new-face make-face make-empty-face face-spec-set custom-declare-face while let my-def-many-faces my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 3 [frame-set-background-mode internal-set-lisp-face-attribute set-face-attribute apply face-spec-set-2 face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 5 [face-attribute face-attr-match-p frame-set-background-mode internal-set-lisp-face-attribute set-face-attribute apply face-spec-set-2 face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 235 [internal-set-lisp-face-attribute set-face-attribute apply face-spec-set-2 face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 18 [facep check-face face-name set-face-attribute-from-resource set-face-attributes-from-resources make-face-x-resource-internal face-spec-recalc frame-set-background-mode internal-set-lisp-face-attribute set-face-attribute apply face-spec-set-2 face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 308 [set-face-attribute-from-resource set-face-attributes-from-resources make-face-x-resource-internal face-spec-recalc frame-set-background-mode internal-set-lisp-face-attribute set-face-attribute apply face-spec-set-2 face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 31 [set-face-attribute apply face-spec-reset-face face-spec-recalc frame-set-background-mode internal-set-lisp-face-attribute set-face-attribute apply face-spec-set-2 face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 53 [set-face-attributes-from-resources make-face-x-resource-internal face-spec-recalc frame-set-background-mode internal-set-lisp-face-attribute set-face-attribute apply face-spec-set-2 face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 7 [make-face-x-resource-internal face-spec-recalc frame-set-background-mode internal-set-lisp-face-attribute set-face-attribute apply face-spec-set-2 face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 2 [face-spec-recalc frame-set-background-mode internal-set-lisp-face-attribute set-face-attribute apply face-spec-set-2 face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [check-face face-name set-face-attribute-from-resource set-face-attributes-from-resources make-face-x-resource-internal face-spec-recalc frame-set-background-mode internal-set-lisp-face-attribute set-face-attribute apply face-spec-set-2 face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [apply face-spec-reset-face face-spec-recalc frame-set-background-mode internal-set-lisp-face-attribute set-face-attribute apply face-spec-set-2 face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 3 [face-name set-face-attribute-from-resource set-face-attributes-from-resources make-face-x-resource-internal face-spec-recalc frame-set-background-mode internal-set-lisp-face-attribute set-face-attribute apply face-spec-set-2 face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 3 [face-spec-choose face-spec-recalc frame-set-background-mode internal-set-lisp-face-attribute set-face-attribute apply face-spec-set-2 face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [set-face-attribute apply face-spec-set-2 face-spec-recalc frame-set-background-mode internal-set-lisp-face-attribute set-face-attribute apply face-spec-set-2 face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [set-face-attribute apply face-spec-set-2 face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 2 [facep check-face face-name set-face-attribute-from-resource set-face-attributes-from-resources make-face-x-resource-internal face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 293 [set-face-attribute apply face-spec-reset-face face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 219 [face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 59 [set-face-attribute-from-resource set-face-attributes-from-resources make-face-x-resource-internal face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 20 [set-face-attributes-from-resources make-face-x-resource-internal face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 4 [face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [face-name set-face-attribute-from-resource set-face-attributes-from-resources make-face-x-resource-internal face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 2 [apply face-spec-reset-face face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 4 [make-face-x-resource-internal face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [apply face-spec-set-2 face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [check-face face-name set-face-attribute-from-resource set-face-attributes-from-resources make-face-x-resource-internal face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [face-spec-set-2 face-spec-recalc face-set-after-frame-default x-show-tip let list let my-bench-x-tip "#<subr eval-region>" apply endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [forward-sexp backward-sexp clojure-backward-logical-sexp cider--make-result-overlay endless/eval-overlay "#<lambda -0x120930d847119138>" apply eval-region elisp--eval-defun "#<compiled 0x1fa5d1dc39debc9e>" apply eval-defun funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [eval format-mode-line sml/fill-width-available sml/generate-minor-modes eval redisplay_internal\ \(C\ function\) nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 ["#<compiled 0xa721f0ff64839>" apply timer-event-handler nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 ["#<compiled 0xde148383988e753>" mapatoms smex-detect-new-commands smex funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 4 [mapatoms smex-detect-new-commands smex funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 ["#<compiled -0x3ac6ecab74afd64>" mapc ido-set-matches-1 ido-set-matches ido-read-internal "#<compiled 0xd9710a97778ae65>" apply ido-completing-read@ido-cr+-replace apply ido-completing-read smex-completing-read smex-read-and-run smex funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [ido-set-matches-1 ido-set-matches ido-read-internal "#<compiled 0xd9710a97778ae65>" apply ido-completing-read@ido-cr+-replace apply ido-completing-read smex-completing-read smex-read-and-run smex funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 ["#<compiled -0x3ac6ecab74afd64>" mapc ido-set-matches-1 ido-set-matches "#<compiled 0x166983c3be51b56a>" apply ido-exhibit read-from-minibuffer ido-read-internal "#<compiled 0xd9710a97778ae65>" apply ido-completing-read@ido-cr+-replace apply ido-completing-read smex-completing-read smex-read-and-run smex funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 16 [mapc ido-set-matches-1 ido-set-matches "#<compiled 0x166983c3be51b56a>" apply ido-exhibit read-from-minibuffer ido-read-internal "#<compiled 0xd9710a97778ae65>" apply ido-completing-read@ido-cr+-replace apply ido-completing-read smex-completing-read smex-read-and-run smex funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 5 [ido-completions "#<compiled 0x166983c3be51b56a>" apply ido-exhibit read-from-minibuffer ido-read-internal "#<compiled 0xd9710a97778ae65>" apply ido-completing-read@ido-cr+-replace apply ido-completing-read smex-completing-read smex-read-and-run smex funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [read-from-minibuffer ido-read-internal "#<compiled 0xd9710a97778ae65>" apply ido-completing-read@ido-cr+-replace apply ido-completing-read smex-completing-read smex-read-and-run smex funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 34 [ido-read-internal "#<compiled 0xd9710a97778ae65>" apply ido-completing-read@ido-cr+-replace apply ido-completing-read smex-completing-read smex-read-and-run smex funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 10 [mapcar ido-find-common-substring ido-set-common-completion "#<compiled 0x166983c3be51b56a>" apply ido-exhibit read-from-minibuffer ido-read-internal "#<compiled 0xd9710a97778ae65>" apply ido-completing-read@ido-cr+-replace apply ido-completing-read smex-completing-read smex-read-and-run smex funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [apply ido-tidy read-from-minibuffer ido-read-internal "#<compiled 0xd9710a97778ae65>" apply ido-completing-read@ido-cr+-replace apply ido-completing-read smex-completing-read smex-read-and-run smex funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [ido-word-matching-substring mapcar ido-find-common-substring ido-set-common-completion "#<compiled 0x166983c3be51b56a>" apply ido-exhibit read-from-minibuffer ido-read-internal "#<compiled 0xd9710a97778ae65>" apply ido-completing-read@ido-cr+-replace apply ido-completing-read smex-completing-read smex-read-and-run smex funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [self-insert-command funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute read-from-minibuffer ido-read-internal "#<compiled 0xd9710a97778ae65>" apply ido-completing-read@ido-cr+-replace apply ido-completing-read smex-completing-read smex-read-and-run smex funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [profiler-report funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute execute-extended-command smex-read-and-run smex funcall-interactively "#<subr call-interactively>" apply call-interactively@ido-cr+-record-current-command apply call-interactively command-execute nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] 1 [Automatic\ GC] 188)) (24250 9524 179007 803000) nil]
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-12 4:30 bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined Clément Pit-Claudel
@ 2020-05-12 6:42 ` martin rudalics
2020-05-12 11:30 ` Clément Pit-Claudel
2020-05-12 15:27 ` Eli Zaretskii
` (2 subsequent siblings)
3 siblings, 1 reply; 57+ messages in thread
From: martin rudalics @ 2020-05-12 6:42 UTC (permalink / raw)
To: Clément Pit-Claudel, 41200
> Is seems that defining a face makes x-show-tip a tiny bit slower, but
> these effects stack.
Please try with 'tooltip-reuse-hidden-frame' non-nil (there's nothing we
can do about a session's first tooltip appearance, though).
martin
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-12 6:42 ` martin rudalics
@ 2020-05-12 11:30 ` Clément Pit-Claudel
2020-05-12 15:12 ` martin rudalics
0 siblings, 1 reply; 57+ messages in thread
From: Clément Pit-Claudel @ 2020-05-12 11:30 UTC (permalink / raw)
To: martin rudalics, 41200
On 12/05/2020 02.42, martin rudalics wrote:
>> Is seems that defining a face makes x-show-tip a tiny bit slower, but
>> these effects stack.
>
> Please try with 'tooltip-reuse-hidden-frame' non-nil (there's nothing we
> can do about a session's first tooltip appearance, though).
I'm not seeing a difference here. I used this code to test:
(defun my-def-many-faces (nfaces)
(dotimes (i nfaces)
(custom-declare-face
(intern (format "my-face-%d" i))
'((t)) "A face."
:group 'basic-faces)))
(defun my-bench-x-tip (nfaces)
(setq x-gtk-use-system-tooltips nil
tooltip-reuse-hidden-frame t)
(my-def-many-faces nfaces)
(benchmark-run 1 (x-show-tip "Test" (selected-frame) nil 5 nil nil)))
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-12 11:30 ` Clément Pit-Claudel
@ 2020-05-12 15:12 ` martin rudalics
2020-05-12 17:19 ` Clément Pit-Claudel
0 siblings, 1 reply; 57+ messages in thread
From: martin rudalics @ 2020-05-12 15:12 UTC (permalink / raw)
To: Clément Pit-Claudel, 41200
[-- Attachment #1: Type: text/plain, Size: 311 bytes --]
>> Please try with 'tooltip-reuse-hidden-frame' non-nil (there's nothing we
>> can do about a session's first tooltip appearance, though).
>
> I'm not seeing a difference here. I used this code to test:
Looks like a devastating bug in the GTK builds. Can you try with the
attached patch?
Thanks, martin
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: tooltip-hide.diff --]
[-- Type: text/x-patch; name="tooltip-hide.diff", Size: 458 bytes --]
diff --git a/src/xfns.c b/src/xfns.c
index a5431aa890..a8cdd66b57 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -6748,7 +6748,8 @@ x_hide_tip (bool delete)
/* Reset tip_last_frame, it will be reassigned when showing the
next GTK+ system tooltip. */
- tip_last_frame = Qnil;
+ if (x_gtk_use_system_tooltips)
+ tip_last_frame = Qnil;
/* Now look whether there's an Emacs tip around. */
if (FRAMEP (tip_frame))
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-12 4:30 bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined Clément Pit-Claudel
2020-05-12 6:42 ` martin rudalics
@ 2020-05-12 15:27 ` Eli Zaretskii
2020-05-13 2:41 ` Clément Pit-Claudel
2020-05-15 14:03 ` Stefan Monnier
2021-04-06 6:35 ` Jashank Jeremy
3 siblings, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2020-05-12 15:27 UTC (permalink / raw)
To: Clément Pit-Claudel; +Cc: 41200
> Resent-From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Original-Sender: "Debbugs-submit" <debbugs-submit-bounces@debbugs.gnu.org>
> Resent-CC: bug-gnu-emacs@gnu.org
> Resent-Sender: help-debbugs@gnu.org
> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Tue, 12 May 2020 00:30:23 -0400
>
>
> [1:text/plain Hide]
>
> Hi all,
>
> (defun my-def-many-faces (nfaces)
> (dotimes (i nfaces)
> (custom-declare-face
> (intern (format "my-face-%d" i))
> '((t)) "A face."
> :group 'basic-faces)))
>
> (defun my-bench-x-tip (nfaces)
> (setq x-gtk-use-system-tooltips nil)
> (my-def-many-faces nfaces)
> (benchmark-run 1 (x-show-tip "Test" (selected-frame) nil 5 nil nil)))
>
> (my-bench-x-tip 100) ;; ⇒ (0.035934318 1 0.015908304000000012)
> (my-bench-x-tip 200) ;; ⇒ (0.049593474 1 0.01508625500000002)
> (my-bench-x-tip 300) ;; ⇒ (0.094929297 2 0.03376510099999999)
> (my-bench-x-tip 400) ;; ⇒ (0.094900665 2 0.03254889999999999)
> (my-bench-x-tip 500) ;; ⇒ (0.118183442 2 0.03218763600000002)
> (my-bench-x-tip 600) ;; ⇒ (0.154759438 3 0.04923829399999996)
> (my-bench-x-tip 700) ;; ⇒ (0.183241646 3 0.04901039700000004)
> (my-bench-x-tip 800) ;; ⇒ (0.212218872 3 0.050182316999999976)
> (my-bench-x-tip 900) ;; ⇒ (0.248743542 3 0.04915146899999995)
> (my-bench-x-tip 1000) ;; ⇒ (0.29221963 3 0.04943874300000006)
> (my-bench-x-tip 1100) ;; ⇒ (0.334084605 3 0.05403986499999991)
> (my-bench-x-tip 1200) ;; ⇒ (0.397292289 4 0.06869684599999992)
> (my-bench-x-tip 1300) ;; ⇒ (0.442873256 4 0.06865671799999995)
> (my-bench-x-tip 1400) ;; ⇒ (0.492474982 4 0.06888139900000001)
> (my-bench-x-tip 1500) ;; ⇒ (0.579180262 5 0.08583425400000011)
> (my-bench-x-tip 1600) ;; ⇒ (0.63504114 5 0.08973981699999989)
> (my-bench-x-tip 1700) ;; ⇒ (0.723722857 5 0.09094433899999999)
> (my-bench-x-tip 1800) ;; ⇒ (0.791952279 5 0.08777533800000015)
> (my-bench-x-tip 1900) ;; ⇒ (0.902377982 6 0.10768666300000018)
> (my-bench-x-tip 2000) ;; ⇒ (0.998815784 6 0.11384837999999986)
>
> Be sure to run it in emacs -q, not emacs -Q, because emacs -Q ignores X resources and hence skips the body of make-face-x-resource-internal, which contributes greatly to the issue.
> For some reasons the effects are a bit worse in my config — roughly a factor 3 to 5 (I have 600 faces defined, and each tooltip takes .5s to display). The profiles below suggest that face-spec-set-2 is called in my config, but not in my repro, which could explain part of the difference.
>
> This is what the profile in emacs -q looks like:
>
> - command-execute 1742 97%
> - call-interactively 1742 97%
> - funcall-interactively 1720 96%
> - eval-defun 1711 95%
> - elisp--eval-defun 1711 95%
> - eval-region 1711 95%
> - let 1711 95%
> - list 1711 95%
> - let 1711 95%
> - x-show-tip 1708 95%
> - face-set-after-frame-default 1708 95%
> - face-spec-recalc 1654 92%
> - make-face-x-resource-internal 1414 78%
> - set-face-attributes-from-resources 1413 78%
> - set-face-attribute-from-resource 1394 77%
> - face-name 1353 75%
> - check-face 1348 75%
> facep 1344 75%
If you look at internal-lisp-face-p, which is the workhorse of facep,
you will see that it does the moral equivalent of
(assq FACE (frame-face-alist))
(The code is actually in a subroutine called by internal-lisp-face-p.)
Which means face-set-after-frame-default, which loops over all of the
faces, runs with O(n²) complexity in the number of faces.
So I think if we want to support such large amounts of faces, we
should not store them in alists, but in a more efficient data
structure.
> Configured using:
> 'configure -C'
>
> Configured features:
> XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY
> INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF
> ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS LIBSYSTEMD
> JSON PDUMPER LCMS2 GMP
No Emacs version information?
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-12 15:12 ` martin rudalics
@ 2020-05-12 17:19 ` Clément Pit-Claudel
2020-05-12 17:42 ` martin rudalics
0 siblings, 1 reply; 57+ messages in thread
From: Clément Pit-Claudel @ 2020-05-12 17:19 UTC (permalink / raw)
To: martin rudalics, 41200
On 12/05/2020 11.12, martin rudalics wrote:
>>> Please try with 'tooltip-reuse-hidden-frame' non-nil (there's nothing we
>>> can do about a session's first tooltip appearance, though).
>>
>> I'm not seeing a difference here. I used this code to test:
>
> Looks like a devastating bug in the GTK builds. Can you try with the
> attached patch?
>
> Thanks, martin
Indeed, with tooltip-reuse-hidden-frame t and your patch, creating a tooltip is instantaneous (after the first tooltip is created)
The docstring suggests that with this option, results won't always be correct. Is there a chance that we could rebuild the frame when needed and then make that option the default?
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-12 17:19 ` Clément Pit-Claudel
@ 2020-05-12 17:42 ` martin rudalics
2020-05-12 17:58 ` Eli Zaretskii
0 siblings, 1 reply; 57+ messages in thread
From: martin rudalics @ 2020-05-12 17:42 UTC (permalink / raw)
To: Clément Pit-Claudel, 41200
> Indeed, with tooltip-reuse-hidden-frame t and your patch, creating a
> tooltip is instantaneous (after the first tooltip is created)
Eli, any problems to fix this in Emacs 27?
> The docstring suggests that with this option, results won't always be
> correct. Is there a chance that we could rebuild the frame when
> needed and then make that option the default?
It depends on what "when needed" stands for. Basically, the results may
be incorrect when some sort of face change happens. But, as I recently
mentioned in another thread, the code not reusing a hidden frame already
fails picking up an internal border face specified via
(set-face-background 'internal-border "red")
so such annoyances are already present in the default code.
In principle, you can always add or remove some non-position-specifying
alist entry and the tooltip frame will be recreated from scratch. So if
we can identify the "when needed", it should be easy to add such an
entry and remove it as soon as the next tooltip frame has been created.
martin
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-12 17:42 ` martin rudalics
@ 2020-05-12 17:58 ` Eli Zaretskii
2020-05-13 14:58 ` martin rudalics
0 siblings, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2020-05-12 17:58 UTC (permalink / raw)
To: martin rudalics; +Cc: cpitclaudel, 41200
> From: martin rudalics <rudalics@gmx.at>
> Date: Tue, 12 May 2020 19:42:52 +0200
>
> > Indeed, with tooltip-reuse-hidden-frame t and your patch, creating a
> > tooltip is instantaneous (after the first tooltip is created)
>
> Eli, any problems to fix this in Emacs 27?
No, please go ahead.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-12 15:27 ` Eli Zaretskii
@ 2020-05-13 2:41 ` Clément Pit-Claudel
2020-05-13 14:58 ` martin rudalics
2020-05-15 11:05 ` Eli Zaretskii
0 siblings, 2 replies; 57+ messages in thread
From: Clément Pit-Claudel @ 2020-05-13 2:41 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 41200
[-- Attachment #1: Type: text/plain, Size: 10595 bytes --]
On 12/05/2020 11.27, Eli Zaretskii wrote:
> (The code is actually in a subroutine called by internal-lisp-face-p.)
> Which means face-set-after-frame-default, which loops over all of the
> faces, runs with O(n²) complexity in the number of faces.
>
> So I think if we want to support such large amounts of faces, we
> should not store them in alists, but in a more efficient data
> structure.
Indeed, you're completely right; thanks! Replacing face_alist and Vface_new_frame_defaults with hash tables makes the worst example about 10 times faster, and with that change tooltips now take 30 to 50ms to display instead of 500-600ms in my real-life use case (my usual config). I have attached a patch.
I left a few questions in the code; I hope that's OK. I have a few more questions that are not part of the code:
* I removed the function frame-face-alist and changed the type of the variable face-new-frame-defaults. Both were documented as internal. Should I add an ELisp implementation of frame-face-alist for compatibility? (It wouldn't be a perfect shim, since modifying its return value wouldn't do the same). For face-new-frame-defaults it's a bit trickier, since the variable now holds a hash table. Should I change its name to make the change obvious, at least?
* The name face_hash isn't ideal, since there's already a distinct notion of face hashes (hash codes). Can you think of a better name?
* I imagine that this change needs to be advertised somewhere, but I'm not sure where; NEWS?
Lastly, do the following new profiles suggest other opportunities for improvement?
- ... 499 52%
Automatic GC 499 52%
- command-execute 454 47%
- call-interactively 454 47%
- funcall-interactively 433 45%
- eval-defun 427 44%
- elisp--eval-defun 427 44%
- eval-region 426 44%
- my-bench-x-tip 426 44%
- let 406 42%
- list 406 42%
- let 406 42%
- x-show-tip 390 40%
- face-set-after-frame-default 387 40%
- face-spec-recalc 374 39%
- make-face-x-resource-internal 296 30%
- set-face-attributes-from-resources 273 28%
- set-face-attribute-from-resource 219 22%
+ face-name 65 6%
+ face-spec-reset-face 62 6%
+ face-spec-set-2 6 0%
+ face-spec-choose 2 0%
+ face-list 2 0%
+ frame-windows-min-size 1 0%
+ my-def-many-faces 20 2%
+ end-of-defun 1 0%
+ execute-extended-command 6 0%
+ byte-code 21 2%
+ redisplay_internal (C function) 2 0%
tooltip-hide 1 0%
- command-execute 768 80%
- call-interactively 768 80%
- apply 768 80%
- call-interactively@ido-cr+-record-current-command 768 80%
- apply 768 80%
- #<subr call-interactively> 768 80%
- funcall-interactively 768 80%
- eval-defun 715 74%
- apply 713 74%
- #<compiled 0x151e3cbebf653c17> 712 74%
- elisp--eval-defun 712 74%
- eval-region 709 74%
- apply 709 74%
- #<lambda 0x32ad1cc4311e0c0> 709 74%
- endless/eval-overlay 709 74%
- apply 709 74%
- #<subr eval-region> 707 74%
- my-bench-x-tip 707 74%
- let 689 72%
- list 689 72%
- let 689 72%
- x-show-tip 676 70%
- face-set-after-frame-default 674 70%
- face-spec-recalc 660 69%
- face-spec-set-2 350 36%
- apply 348 36%
- set-face-attribute 342 35%
- internal-set-lisp-face-attribute 342 35%
- frame-set-background-mode 331 34%
- face-spec-recalc 284 29%
- make-face-x-resource-internal 235 24%
- set-face-attributes-from-resources 216 22%
- set-face-attribute-from-resource 174 18%
- face-name 36 3%
+ check-face 21 2%
+ face-spec-reset-face 40 4%
+ face-spec-set-2 4 0%
+ face-attr-match-p 24 2%
face-spec-choose 1 0%
+ face-list 1 0%
- make-face-x-resource-internal 248 25%
- set-face-attributes-from-resources 215 22%
- set-face-attribute-from-resource 169 17%
+ face-name 36 3%
+ face-spec-reset-face 54 5%
+ face-spec-choose 2 0%
+ face-list 1 0%
+ my-def-many-faces 18 1%
+ beginning-of-defun 1 0%
end-of-defun 1 0%
+ #<lambda 0x159f62efd027a> 2 0%
+ smex 53 5%
- ... 182 19%
Automatic GC 182 19%
+ redisplay_internal (C function) 3 0%
Also, since the GC seems to be a significant part, here's a memory profile:
- command-execute 305,314,261 99%
- call-interactively 305,314,261 99%
- funcall-interactively 305,262,257 99%
- eval-defun 303,318,241 98%
- elisp--eval-defun 303,317,185 98%
- eval-region 303,296,332 98%
- my-bench-x-tip 303,295,276 98%
- let 273,049,377 89%
- list 273,049,377 89%
- let 273,049,377 89%
- x-show-tip 177,538,262 57%
- face-set-after-frame-default 175,519,190 57%
- face-spec-recalc 174,935,046 57%
- make-face-x-resource-internal 138,435,960 45%
+ set-face-attributes-from-resources 138,407,728 45%
+ face-spec-reset-face 36,126,838 11%
+ face-spec-choose 74,360 0%
+ face-spec-set-2 21,216 0%
+ face-list 554,400 0%
+ frame-windows-min-size 7,676 0%
+ run-at-time 4,352 0%
setq 4,224 0%
+ float-time 3,888 0%
+ my-def-many-faces 30,245,899 9%
+ internal-macroexpand-for-load 1,056 0%
+ end-of-defun 4,160 0%
+ beginning-of-defun 2,112 0%
+ execute-extended-command 1,944,016 0%
+ byte-code 52,004 0%
+ redisplay_internal (C function) 1,427,117 0%
> No Emacs version information?
Woops. Sorry! GNU Emacs 28.0.50 (build 10, x86_64-pc-linux-gnu, GTK+ Version 3.22.30, cairo version 1.15.10) of 2020-05-10
Thanks again for the pointers,
Clément.
[-- Attachment #2: 0001-Store-frame-faces-in-hash-tables-instead-of-alists.patch --]
[-- Type: text/x-patch, Size: 12556 bytes --]
From 6d3d5d94189ae31fdbc29b2db0707d6b13a5c362 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Pit-Claudel?= <clement.pitclaudel@live.com>
Date: Tue, 12 May 2020 21:48:32 -0400
Subject: [PATCH] Store frame faces in hash tables instead of alists
* lisp/emacs-lisp/edebug.el (edebug-eval-defun):
* lisp/progmodes/elisp-mode.el (elisp--eval-defun-1):
* lisp/faces.el (face-list):
* lisp/frame.el (frame-set-background-mode): Update to work with hash
tables instead of alists.
* src/frame.h (struct frame): Remove face_alist, add face_hash.
(fset_face_alist): Remove.
(fset_face_hash): New function.
* src/frame.c (make_frame): Initialize f->face_hash.
(Fmake_terminal_frame): Update to work with hash tables instead of
alists.
* src/xfaces.c (lface_from_face_name_no_resolve):
(Finternal_make_lisp_face):
(update_face_from_frame_parameter): Update to work with hash tables
instead of alists.
* src/xfaces.c (Fframe_face_hash): New function.
(Fframe_face_alist): Remove.
(init_xfaces): Compute face IDs from they face property, not from
their position in face_alist.
(syms_of_xfaces): Remove frame_face_alist, add frame_face_hash;
change Vface_new_frame_defaults into a hash table.
---
lisp/emacs-lisp/edebug.el | 3 +-
lisp/faces.el | 4 ++-
lisp/frame.el | 2 +-
lisp/progmodes/elisp-mode.el | 3 +-
src/frame.c | 21 ++++++++----
src/frame.h | 8 ++---
src/xfaces.c | 64 ++++++++++++++++++++----------------
7 files changed, 59 insertions(+), 46 deletions(-)
diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index 78461185d3..97ca964eef 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -488,8 +488,7 @@ edebug-eval-defun
(set-default (nth 1 form) (eval (nth 2 form) lexical-binding)))
((eq (car form) 'defface)
;; Reset the face.
- (setq face-new-frame-defaults
- (assq-delete-all (nth 1 form) face-new-frame-defaults))
+ (remhash (nth 1 form) face-new-frame-defaults)
(put (nth 1 form) 'face-defface-spec nil)
(put (nth 1 form) 'face-documentation (nth 3 form))
;; See comments in `eval-defun-1' for purpose of code below
diff --git a/lisp/faces.el b/lisp/faces.el
index e707f6f4b6..1764f3da75 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -179,7 +179,9 @@ face-font-registry-alternatives
(defun face-list ()
"Return a list of all defined faces."
- (mapcar #'car face-new-frame-defaults))
+ (let ((faces nil))
+ (maphash (lambda (face _) (push face faces)) face-new-frame-defaults)
+ (nreverse faces)))
(defun make-face (face)
"Define a new face with name FACE, a symbol.
diff --git a/lisp/frame.el b/lisp/frame.el
index 6c2f774709..bab3c2a1c2 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -1227,7 +1227,7 @@ frame-set-background-mode
;; during startup with -rv on the command
;; line for the initial frame, because frames
;; are not recorded in the pdump file.
- (assq face (frame-face-alist))
+ (gethash face (frame-face-hash))
(face-spec-match-p face
(face-user-default-spec face)
;; FIXME: why selected-frame and
diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index b737134f90..dc8688c864 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -1304,8 +1304,7 @@ elisp--eval-defun-1
((eq (car form) 'custom-declare-face)
;; Reset the face.
(let ((face-symbol (eval (nth 1 form) lexical-binding)))
- (setq face-new-frame-defaults
- (assq-delete-all face-symbol face-new-frame-defaults))
+ (remhash face-new-frame-defaults face-symbol)
(put face-symbol 'face-defface-spec nil)
(put face-symbol 'face-override-spec nil))
form)
diff --git a/src/frame.c b/src/frame.c
index c871e4fd99..4423965a01 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -946,6 +946,11 @@ make_frame (bool mini_p)
rw->total_lines = mini_p ? 9 : 10;
rw->pixel_height = rw->total_lines * FRAME_LINE_HEIGHT (f);
+ // QUESTION: is this where this should be initialized?
+ fset_face_hash
+ (f, make_hash_table(hashtest_eq, DEFAULT_HASH_SIZE, DEFAULT_REHASH_SIZE,
+ DEFAULT_REHASH_THRESHOLD, Qnil, Qnil));
+
if (mini_p)
{
mw->top_line = rw->total_lines;
@@ -1254,7 +1259,7 @@ DEFUN ("make-terminal-frame", Fmake_terminal_frame, Smake_terminal_frame,
{
struct frame *f;
struct terminal *t = NULL;
- Lisp_Object frame, tem;
+ Lisp_Object frame;
struct frame *sf = SELECTED_FRAME ();
#ifdef MSDOS
@@ -1336,14 +1341,16 @@ DEFUN ("make-terminal-frame", Fmake_terminal_frame, Smake_terminal_frame,
store_in_alist (&parms, Qminibuffer, Qt);
Fmodify_frame_parameters (frame, parms);
- /* Make the frame face alist be frame-specific, so that each
+ /* Make the frame face hash be frame-specific, so that each
frame could change its face definitions independently. */
- fset_face_alist (f, Fcopy_alist (sf->face_alist));
- /* Simple Fcopy_alist isn't enough, because we need the contents of
- the vectors which are the CDRs of associations in face_alist to
+ fset_face_hash (f, Fcopy_hash_table (sf->face_hash));
+ /* Simple copy_hash_table isn't enough, because we need the contents of
+ the vectors which are the values in face_hash to
be copied as well. */
- for (tem = f->face_alist; CONSP (tem); tem = XCDR (tem))
- XSETCDR (XCAR (tem), Fcopy_sequence (XCDR (XCAR (tem))));
+ ptrdiff_t idx = 0;
+ struct Lisp_Hash_Table *table = XHASH_TABLE(f->face_hash);
+ for (idx = 0; idx < table->count; ++idx)
+ set_hash_value_slot (table, idx, Fcopy_sequence(HASH_VALUE (table, idx)));
f->can_set_window_size = true;
f->after_make_frame = true;
diff --git a/src/frame.h b/src/frame.h
index 476bac67fa..0a324414a6 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -158,8 +158,8 @@ #define EMACS_FRAME_H
There are four additional elements of nil at the end, to terminate. */
Lisp_Object menu_bar_items;
- /* Alist of elements (FACE-NAME . FACE-VECTOR-DATA). */
- Lisp_Object face_alist;
+ /* Hash table of FACE-NAME keys and FACE-VECTOR-DATA values. */
+ Lisp_Object face_hash;
/* A vector that records the entire structure of this frame's menu bar.
For the format of the data, see extensive comments in xmenu.c.
@@ -661,9 +661,9 @@ fset_condemned_scroll_bars (struct frame *f, Lisp_Object val)
f->condemned_scroll_bars = val;
}
INLINE void
-fset_face_alist (struct frame *f, Lisp_Object val)
+fset_face_hash (struct frame *f, Lisp_Object val)
{
- f->face_alist = val;
+ f->face_hash = val;
}
#if defined (HAVE_WINDOW_SYSTEM)
INLINE void
diff --git a/src/xfaces.c b/src/xfaces.c
index 7d7aff95c1..cccf0e4852 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -1843,13 +1843,11 @@ lface_from_face_name_no_resolve (struct frame *f, Lisp_Object face_name,
Lisp_Object lface;
if (f)
- lface = assq_no_quit (face_name, f->face_alist);
+ lface = Fgethash (face_name, f->face_hash, Qnil);
else
- lface = assq_no_quit (face_name, Vface_new_frame_defaults);
+ lface = Fgethash (face_name, Vface_new_frame_defaults, Qnil);
- if (CONSP (lface))
- lface = XCDR (lface);
- else if (signal_p)
+ if (signal_p && NILP (lface))
signal_error ("Invalid face", face_name);
check_lface (lface);
@@ -2736,8 +2734,7 @@ DEFUN ("internal-make-lisp-face", Finternal_make_lisp_face,
{
global_lface = make_vector (LFACE_VECTOR_SIZE, Qunspecified);
ASET (global_lface, 0, Qface);
- Vface_new_frame_defaults = Fcons (Fcons (face, global_lface),
- Vface_new_frame_defaults);
+ Fputhash(face, global_lface, Vface_new_frame_defaults);
/* Assign the new Lisp face a unique ID. The mapping from Lisp
face id to Lisp face is given by the vector lface_id_to_name.
@@ -2763,7 +2760,7 @@ DEFUN ("internal-make-lisp-face", Finternal_make_lisp_face,
{
lface = make_vector (LFACE_VECTOR_SIZE, Qunspecified);
ASET (lface, 0, Qface);
- fset_face_alist (f, Fcons (Fcons (face, lface), f->face_alist));
+ Fputhash (face, lface, f->face_hash);
}
else
for (i = 1; i < LFACE_VECTOR_SIZE; ++i)
@@ -3508,7 +3505,7 @@ update_face_from_frame_parameter (struct frame *f, Lisp_Object param,
/* If there are no faces yet, give up. This is the case when called
from Fx_create_frame, and we do the necessary things later in
face-set-after-frame-defaults. */
- if (NILP (f->face_alist))
+ if (XFIXNAT(Fhash_table_count(f->face_hash)) == 0)
return;
if (EQ (param, Qforeground_color))
@@ -4174,14 +4171,14 @@ DEFUN ("internal-lisp-face-empty-p", Finternal_lisp_face_empty_p,
return i == LFACE_VECTOR_SIZE ? Qt : Qnil;
}
-
-DEFUN ("frame-face-alist", Fframe_face_alist, Sframe_face_alist,
+// QUESTION: Should I add an ELisp version of frame-face-hash?
+DEFUN ("frame-face-hash", Fframe_face_hash, Sframe_face_hash,
0, 1, 0,
doc: /* Return an alist of frame-local faces defined on FRAME.
For internal use only. */)
(Lisp_Object frame)
{
- return decode_live_frame (frame)->face_alist;
+ return decode_live_frame (frame)->face_hash;
}
@@ -6678,30 +6675,37 @@ DEFUN ("show-face-resources", Fshow_face_resources, Sshow_face_resources,
#ifdef HAVE_PDUMPER
/* All the faces defined during loadup are recorded in
- face-new-frame-defaults, with the last face first in the list. We
- need to set next_lface_id to the next face ID number, so that any
- new faces defined in this session will have face IDs different from
- those defined during loadup. We also need to set up the
- lface_id_to_name[] array for the faces that were defined during
- loadup. */
+ face-new-frame-defaults. We need to set next_lface_id to the next
+ face ID number, so that any new faces defined in this session will
+ have face IDs different from those defined during loadup. We also
+ need to set up the lface_id_to_name[] array for the faces that were
+ defined during loadup. */
void
init_xfaces (void)
{
- if (CONSP (Vface_new_frame_defaults))
+ int nfaces = XFIXNAT(Fhash_table_count (Vface_new_frame_defaults));
+ if (nfaces > 0)
{
/* Allocate the lface_id_to_name[] array. */
- lface_id_to_name_size = next_lface_id =
- XFIXNAT (Flength (Vface_new_frame_defaults));
+ lface_id_to_name_size = next_lface_id = nfaces;
lface_id_to_name = xnmalloc (next_lface_id, sizeof *lface_id_to_name);
/* Store the faces. */
- Lisp_Object tail;
- int i = next_lface_id - 1;
- for (tail = Vface_new_frame_defaults; CONSP (tail); tail = XCDR (tail))
+ struct Lisp_Hash_Table* table = XHASH_TABLE(Vface_new_frame_defaults);
+ for (ptrdiff_t idx = 0; idx < nfaces; ++idx)
{
- Lisp_Object lface = XCAR (tail);
- eassert (i >= 0);
- lface_id_to_name[i--] = XCAR (lface);
+ Lisp_Object lface = HASH_KEY(table, idx);
+ Lisp_Object face_id = Fget (lface, Qface);
+ // FIXME why is (get 'tab-line 'face) 0?
+ if (!FIXNATP (face_id))
+ // FIXME: I'm not sure what to do in this case
+ printf("Face %s has no id\n", SDATA(SYMBOL_NAME (lface)));
+ else
+ {
+ int id = XFIXNAT(face_id);
+ eassert (id >= 0);
+ lface_id_to_name[id] = lface;
+ }
}
}
face_attr_sym[0] = Qface;
@@ -6855,7 +6859,7 @@ syms_of_xfaces (void)
defsubr (&Sinternal_copy_lisp_face);
defsubr (&Sinternal_merge_in_global_face);
defsubr (&Sface_font);
- defsubr (&Sframe_face_alist);
+ defsubr (&Sframe_face_hash);
defsubr (&Sdisplay_supports_face_attributes_p);
defsubr (&Scolor_distance);
defsubr (&Sinternal_set_font_selection_order);
@@ -6881,7 +6885,9 @@ syms_of_xfaces (void)
DEFVAR_LISP ("face-new-frame-defaults", Vface_new_frame_defaults,
doc: /* List of global face definitions (for internal use only.) */);
- Vface_new_frame_defaults = Qnil;
+ Vface_new_frame_defaults =
+ make_hash_table (hashtest_eq, DEFAULT_HASH_SIZE, DEFAULT_REHASH_SIZE,
+ DEFAULT_REHASH_THRESHOLD, Qnil, Qnil);
DEFVAR_LISP ("face-default-stipple", Vface_default_stipple,
doc: /* Default stipple pattern used on monochrome displays.
--
2.17.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-12 17:58 ` Eli Zaretskii
@ 2020-05-13 14:58 ` martin rudalics
0 siblings, 0 replies; 57+ messages in thread
From: martin rudalics @ 2020-05-13 14:58 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: cpitclaudel, 41200
> No, please go ahead.
Installed meanwhile.
Thanks, martin
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-13 2:41 ` Clément Pit-Claudel
@ 2020-05-13 14:58 ` martin rudalics
2020-05-13 15:13 ` Clément Pit-Claudel
2020-05-15 11:05 ` Eli Zaretskii
1 sibling, 1 reply; 57+ messages in thread
From: martin rudalics @ 2020-05-13 14:58 UTC (permalink / raw)
To: Clément Pit-Claudel, Eli Zaretskii; +Cc: 41200
> Indeed, you're completely right; thanks! Replacing face_alist and
> Vface_new_frame_defaults with hash tables makes the worst example
> about 10 times faster, and with that change tooltips now take 30 to
> 50ms to display instead of 500-600ms in my real-life use case (my
> usual config). I have attached a patch.
GCC throws the following error here:
CC frame.o
In file included from ../../src/lisp.h:954,
from ../../src/frame.c:29:
../../src/frame.c: In function ‘make_frame’:
./globals.h:6365:14: error: incompatible type for argument 6 of ‘make_hash_table’
#define Qnil builtin_lisp_symbol (0)
^~~~~~~~~~~~~~~~~~~~~~~
../../src/frame.c:952:57: note: in expansion of macro ‘Qnil’
DEFAULT_REHASH_THRESHOLD, Qnil, Qnil));
^~~~
In file included from ../../src/conf_post.h:39,
from ./config.h:2270,
from ../../src/frame.c:20:
../../src/lisp.h:3661:43: note: expected ‘_Bool’ but argument is of type ‘Lisp_Object’ {aka ‘struct Lisp_Object’}
Lisp_Object, bool);
^~~~
make[1]: *** [Makefile:402: frame.o] Fehler 1
make[1]: *** Es wird auf noch nicht beendete Prozesse gewartet....
make[1]: Verzeichnis „/home/martin/emacs-git/release/obj-gtk/src“ wird verlassen
make: *** [Makefile:424: src] Fehler 2
martin
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-13 14:58 ` martin rudalics
@ 2020-05-13 15:13 ` Clément Pit-Claudel
2020-05-13 17:42 ` martin rudalics
0 siblings, 1 reply; 57+ messages in thread
From: Clément Pit-Claudel @ 2020-05-13 15:13 UTC (permalink / raw)
To: martin rudalics, Eli Zaretskii; +Cc: 41200
[-- Attachment #1: Type: text/plain, Size: 506 bytes --]
On 13/05/2020 10.58, martin rudalics wrote:
>> Indeed, you're completely right; thanks! Replacing face_alist and
>> Vface_new_frame_defaults with hash tables makes the worst example
>> about 10 times faster, and with that change tooltips now take 30 to
>> 50ms to display instead of 500-600ms in my real-life use case (my
>> usual config). I have attached a patch.
>
> GCC throws the following error here:
Woops, thanks. I misread the signature of make_hash_table. I've attached an updated patch.
[-- Attachment #2: 0001-Store-frame-faces-in-hash-tables-instead-of-alists.patch --]
[-- Type: text/x-patch, Size: 12558 bytes --]
From 75d44050a9277c708c69afe09de334ab3c300da1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Pit-Claudel?= <clement.pitclaudel@live.com>
Date: Tue, 12 May 2020 21:48:32 -0400
Subject: [PATCH] Store frame faces in hash tables instead of alists
* lisp/emacs-lisp/edebug.el (edebug-eval-defun):
* lisp/progmodes/elisp-mode.el (elisp--eval-defun-1):
* lisp/faces.el (face-list):
* lisp/frame.el (frame-set-background-mode): Update to work with hash
tables instead of alists.
* src/frame.h (struct frame): Remove face_alist, add face_hash.
(fset_face_alist): Remove.
(fset_face_hash): New function.
* src/frame.c (make_frame): Initialize f->face_hash.
(Fmake_terminal_frame): Update to work with hash tables instead of
alists.
* src/xfaces.c (lface_from_face_name_no_resolve):
(Finternal_make_lisp_face):
(update_face_from_frame_parameter): Update to work with hash tables
instead of alists.
* src/xfaces.c (Fframe_face_hash): New function.
(Fframe_face_alist): Remove.
(init_xfaces): Compute face IDs from they face property, not from
their position in face_alist.
(syms_of_xfaces): Remove frame_face_alist, add frame_face_hash;
change Vface_new_frame_defaults into a hash table.
---
lisp/emacs-lisp/edebug.el | 3 +-
lisp/faces.el | 4 ++-
lisp/frame.el | 2 +-
lisp/progmodes/elisp-mode.el | 3 +-
src/frame.c | 21 ++++++++----
src/frame.h | 8 ++---
src/xfaces.c | 64 ++++++++++++++++++++----------------
7 files changed, 59 insertions(+), 46 deletions(-)
diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index 78461185d3..97ca964eef 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -488,8 +488,7 @@ edebug-eval-defun
(set-default (nth 1 form) (eval (nth 2 form) lexical-binding)))
((eq (car form) 'defface)
;; Reset the face.
- (setq face-new-frame-defaults
- (assq-delete-all (nth 1 form) face-new-frame-defaults))
+ (remhash (nth 1 form) face-new-frame-defaults)
(put (nth 1 form) 'face-defface-spec nil)
(put (nth 1 form) 'face-documentation (nth 3 form))
;; See comments in `eval-defun-1' for purpose of code below
diff --git a/lisp/faces.el b/lisp/faces.el
index e707f6f4b6..1764f3da75 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -179,7 +179,9 @@ face-font-registry-alternatives
(defun face-list ()
"Return a list of all defined faces."
- (mapcar #'car face-new-frame-defaults))
+ (let ((faces nil))
+ (maphash (lambda (face _) (push face faces)) face-new-frame-defaults)
+ (nreverse faces)))
(defun make-face (face)
"Define a new face with name FACE, a symbol.
diff --git a/lisp/frame.el b/lisp/frame.el
index 6c2f774709..bab3c2a1c2 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -1227,7 +1227,7 @@ frame-set-background-mode
;; during startup with -rv on the command
;; line for the initial frame, because frames
;; are not recorded in the pdump file.
- (assq face (frame-face-alist))
+ (gethash face (frame-face-hash))
(face-spec-match-p face
(face-user-default-spec face)
;; FIXME: why selected-frame and
diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index b737134f90..dc8688c864 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -1304,8 +1304,7 @@ elisp--eval-defun-1
((eq (car form) 'custom-declare-face)
;; Reset the face.
(let ((face-symbol (eval (nth 1 form) lexical-binding)))
- (setq face-new-frame-defaults
- (assq-delete-all face-symbol face-new-frame-defaults))
+ (remhash face-new-frame-defaults face-symbol)
(put face-symbol 'face-defface-spec nil)
(put face-symbol 'face-override-spec nil))
form)
diff --git a/src/frame.c b/src/frame.c
index c871e4fd99..7cd43e4fec 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -946,6 +946,11 @@ make_frame (bool mini_p)
rw->total_lines = mini_p ? 9 : 10;
rw->pixel_height = rw->total_lines * FRAME_LINE_HEIGHT (f);
+ // QUESTION: is this where this should be initialized?
+ fset_face_hash
+ (f, make_hash_table(hashtest_eq, DEFAULT_HASH_SIZE, DEFAULT_REHASH_SIZE,
+ DEFAULT_REHASH_THRESHOLD, Qnil, false));
+
if (mini_p)
{
mw->top_line = rw->total_lines;
@@ -1254,7 +1259,7 @@ DEFUN ("make-terminal-frame", Fmake_terminal_frame, Smake_terminal_frame,
{
struct frame *f;
struct terminal *t = NULL;
- Lisp_Object frame, tem;
+ Lisp_Object frame;
struct frame *sf = SELECTED_FRAME ();
#ifdef MSDOS
@@ -1336,14 +1341,16 @@ DEFUN ("make-terminal-frame", Fmake_terminal_frame, Smake_terminal_frame,
store_in_alist (&parms, Qminibuffer, Qt);
Fmodify_frame_parameters (frame, parms);
- /* Make the frame face alist be frame-specific, so that each
+ /* Make the frame face hash be frame-specific, so that each
frame could change its face definitions independently. */
- fset_face_alist (f, Fcopy_alist (sf->face_alist));
- /* Simple Fcopy_alist isn't enough, because we need the contents of
- the vectors which are the CDRs of associations in face_alist to
+ fset_face_hash (f, Fcopy_hash_table (sf->face_hash));
+ /* Simple copy_hash_table isn't enough, because we need the contents of
+ the vectors which are the values in face_hash to
be copied as well. */
- for (tem = f->face_alist; CONSP (tem); tem = XCDR (tem))
- XSETCDR (XCAR (tem), Fcopy_sequence (XCDR (XCAR (tem))));
+ ptrdiff_t idx = 0;
+ struct Lisp_Hash_Table *table = XHASH_TABLE(f->face_hash);
+ for (idx = 0; idx < table->count; ++idx)
+ set_hash_value_slot (table, idx, Fcopy_sequence(HASH_VALUE (table, idx)));
f->can_set_window_size = true;
f->after_make_frame = true;
diff --git a/src/frame.h b/src/frame.h
index 476bac67fa..0a324414a6 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -158,8 +158,8 @@ #define EMACS_FRAME_H
There are four additional elements of nil at the end, to terminate. */
Lisp_Object menu_bar_items;
- /* Alist of elements (FACE-NAME . FACE-VECTOR-DATA). */
- Lisp_Object face_alist;
+ /* Hash table of FACE-NAME keys and FACE-VECTOR-DATA values. */
+ Lisp_Object face_hash;
/* A vector that records the entire structure of this frame's menu bar.
For the format of the data, see extensive comments in xmenu.c.
@@ -661,9 +661,9 @@ fset_condemned_scroll_bars (struct frame *f, Lisp_Object val)
f->condemned_scroll_bars = val;
}
INLINE void
-fset_face_alist (struct frame *f, Lisp_Object val)
+fset_face_hash (struct frame *f, Lisp_Object val)
{
- f->face_alist = val;
+ f->face_hash = val;
}
#if defined (HAVE_WINDOW_SYSTEM)
INLINE void
diff --git a/src/xfaces.c b/src/xfaces.c
index 7d7aff95c1..4f97ba4971 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -1843,13 +1843,11 @@ lface_from_face_name_no_resolve (struct frame *f, Lisp_Object face_name,
Lisp_Object lface;
if (f)
- lface = assq_no_quit (face_name, f->face_alist);
+ lface = Fgethash (face_name, f->face_hash, Qnil);
else
- lface = assq_no_quit (face_name, Vface_new_frame_defaults);
+ lface = Fgethash (face_name, Vface_new_frame_defaults, Qnil);
- if (CONSP (lface))
- lface = XCDR (lface);
- else if (signal_p)
+ if (signal_p && NILP (lface))
signal_error ("Invalid face", face_name);
check_lface (lface);
@@ -2736,8 +2734,7 @@ DEFUN ("internal-make-lisp-face", Finternal_make_lisp_face,
{
global_lface = make_vector (LFACE_VECTOR_SIZE, Qunspecified);
ASET (global_lface, 0, Qface);
- Vface_new_frame_defaults = Fcons (Fcons (face, global_lface),
- Vface_new_frame_defaults);
+ Fputhash(face, global_lface, Vface_new_frame_defaults);
/* Assign the new Lisp face a unique ID. The mapping from Lisp
face id to Lisp face is given by the vector lface_id_to_name.
@@ -2763,7 +2760,7 @@ DEFUN ("internal-make-lisp-face", Finternal_make_lisp_face,
{
lface = make_vector (LFACE_VECTOR_SIZE, Qunspecified);
ASET (lface, 0, Qface);
- fset_face_alist (f, Fcons (Fcons (face, lface), f->face_alist));
+ Fputhash (face, lface, f->face_hash);
}
else
for (i = 1; i < LFACE_VECTOR_SIZE; ++i)
@@ -3508,7 +3505,7 @@ update_face_from_frame_parameter (struct frame *f, Lisp_Object param,
/* If there are no faces yet, give up. This is the case when called
from Fx_create_frame, and we do the necessary things later in
face-set-after-frame-defaults. */
- if (NILP (f->face_alist))
+ if (XFIXNAT(Fhash_table_count(f->face_hash)) == 0)
return;
if (EQ (param, Qforeground_color))
@@ -4174,14 +4171,14 @@ DEFUN ("internal-lisp-face-empty-p", Finternal_lisp_face_empty_p,
return i == LFACE_VECTOR_SIZE ? Qt : Qnil;
}
-
-DEFUN ("frame-face-alist", Fframe_face_alist, Sframe_face_alist,
+// QUESTION: Should I add an ELisp version of frame-face-hash?
+DEFUN ("frame-face-hash", Fframe_face_hash, Sframe_face_hash,
0, 1, 0,
doc: /* Return an alist of frame-local faces defined on FRAME.
For internal use only. */)
(Lisp_Object frame)
{
- return decode_live_frame (frame)->face_alist;
+ return decode_live_frame (frame)->face_hash;
}
@@ -6678,30 +6675,37 @@ DEFUN ("show-face-resources", Fshow_face_resources, Sshow_face_resources,
#ifdef HAVE_PDUMPER
/* All the faces defined during loadup are recorded in
- face-new-frame-defaults, with the last face first in the list. We
- need to set next_lface_id to the next face ID number, so that any
- new faces defined in this session will have face IDs different from
- those defined during loadup. We also need to set up the
- lface_id_to_name[] array for the faces that were defined during
- loadup. */
+ face-new-frame-defaults. We need to set next_lface_id to the next
+ face ID number, so that any new faces defined in this session will
+ have face IDs different from those defined during loadup. We also
+ need to set up the lface_id_to_name[] array for the faces that were
+ defined during loadup. */
void
init_xfaces (void)
{
- if (CONSP (Vface_new_frame_defaults))
+ int nfaces = XFIXNAT(Fhash_table_count (Vface_new_frame_defaults));
+ if (nfaces > 0)
{
/* Allocate the lface_id_to_name[] array. */
- lface_id_to_name_size = next_lface_id =
- XFIXNAT (Flength (Vface_new_frame_defaults));
+ lface_id_to_name_size = next_lface_id = nfaces;
lface_id_to_name = xnmalloc (next_lface_id, sizeof *lface_id_to_name);
/* Store the faces. */
- Lisp_Object tail;
- int i = next_lface_id - 1;
- for (tail = Vface_new_frame_defaults; CONSP (tail); tail = XCDR (tail))
+ struct Lisp_Hash_Table* table = XHASH_TABLE(Vface_new_frame_defaults);
+ for (ptrdiff_t idx = 0; idx < nfaces; ++idx)
{
- Lisp_Object lface = XCAR (tail);
- eassert (i >= 0);
- lface_id_to_name[i--] = XCAR (lface);
+ Lisp_Object lface = HASH_KEY(table, idx);
+ Lisp_Object face_id = Fget (lface, Qface);
+ // FIXME why is (get 'tab-line 'face) 0?
+ if (!FIXNATP (face_id))
+ // FIXME: I'm not sure what to do in this case
+ printf("Face %s has no id\n", SDATA(SYMBOL_NAME (lface)));
+ else
+ {
+ int id = XFIXNAT(face_id);
+ eassert (id >= 0);
+ lface_id_to_name[id] = lface;
+ }
}
}
face_attr_sym[0] = Qface;
@@ -6855,7 +6859,7 @@ syms_of_xfaces (void)
defsubr (&Sinternal_copy_lisp_face);
defsubr (&Sinternal_merge_in_global_face);
defsubr (&Sface_font);
- defsubr (&Sframe_face_alist);
+ defsubr (&Sframe_face_hash);
defsubr (&Sdisplay_supports_face_attributes_p);
defsubr (&Scolor_distance);
defsubr (&Sinternal_set_font_selection_order);
@@ -6881,7 +6885,9 @@ syms_of_xfaces (void)
DEFVAR_LISP ("face-new-frame-defaults", Vface_new_frame_defaults,
doc: /* List of global face definitions (for internal use only.) */);
- Vface_new_frame_defaults = Qnil;
+ Vface_new_frame_defaults =
+ make_hash_table (hashtest_eq, DEFAULT_HASH_SIZE, DEFAULT_REHASH_SIZE,
+ DEFAULT_REHASH_THRESHOLD, Qnil, false);
DEFVAR_LISP ("face-default-stipple", Vface_default_stipple,
doc: /* Default stipple pattern used on monochrome displays.
--
2.17.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-13 15:13 ` Clément Pit-Claudel
@ 2020-05-13 17:42 ` martin rudalics
0 siblings, 0 replies; 57+ messages in thread
From: martin rudalics @ 2020-05-13 17:42 UTC (permalink / raw)
To: Clément Pit-Claudel, Eli Zaretskii; +Cc: 41200
> Woops, thanks. I misread the signature of make_hash_table. I've attached an updated patch.
Thanks. Builds now and I'm running with it.
martin
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-13 2:41 ` Clément Pit-Claudel
2020-05-13 14:58 ` martin rudalics
@ 2020-05-15 11:05 ` Eli Zaretskii
2020-05-15 14:59 ` Clément Pit-Claudel
1 sibling, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2020-05-15 11:05 UTC (permalink / raw)
To: Clément Pit-Claudel; +Cc: 41200
> Cc: 41200@debbugs.gnu.org
> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Tue, 12 May 2020 22:41:24 -0400
>
> > So I think if we want to support such large amounts of faces, we
> > should not store them in alists, but in a more efficient data
> > structure.
>
> Indeed, you're completely right; thanks! Replacing face_alist and Vface_new_frame_defaults with hash tables makes the worst example about 10 times faster, and with that change tooltips now take 30 to 50ms to display instead of 500-600ms in my real-life use case (my usual config). I have attached a patch.
>
> I left a few questions in the code; I hope that's OK. I have a few more questions that are not part of the code:
>
> * I removed the function frame-face-alist and changed the type of the variable face-new-frame-defaults. Both were documented as internal. Should I add an ELisp implementation of frame-face-alist for compatibility? (It wouldn't be a perfect shim, since modifying its return value wouldn't do the same). For face-new-frame-defaults it's a bit trickier, since the variable now holds a hash table. Should I change its name to make the change obvious, at least?
I'd like to keep the old face-new-frame-defaults and frame-face-alist
for compatibility, but mention in the doc strings that they no longer
return modifiable values, and perhaps deprecate them.
> * The name face_hash isn't ideal, since there's already a distinct notion of face hashes (hash codes). Can you think of a better name?
face_hash_table?
> * I imagine that this change needs to be advertised somewhere, but I'm not sure where; NEWS?
Let's think about this after we figured out what changes are needed in
the current functions and variables.
> Lastly, do the following new profiles suggest other opportunities for improvement?
I don't think so, but if the behavior is now linear or sub-linear,
it's the best we can expect, since creating a new frame must walk over
all the faces.
> + // QUESTION: is this where this should be initialized?
Yes, I think so. But do we need to do anything when frame is deleted
as well?
> + fset_face_hash
> + (f, make_hash_table(hashtest_eq, DEFAULT_HASH_SIZE, DEFAULT_REHASH_SIZE,
^^
Our C coding conventions are to leave one space between the function's
name and the opening parenthesis (here and elsewhere in the patch).
> -DEFUN ("frame-face-alist", Fframe_face_alist, Sframe_face_alist,
> +// QUESTION: Should I add an ELisp version of frame-face-hash?
You mean, frame-face-alist, right? Yes, most definitely: I imagine a
lot of code out there uses that, and we wouldn't want to break that.
And I'm not sure we should have it only in Lisp: perhaps we should
maintain the alist as well, and add/remove to/from it when a face is
added or removed in the hash table. Otherwise this change of
internals will have painful effect on packages that use the current
APIs.
> + Lisp_Object lface = HASH_KEY(table, idx);
> + Lisp_Object face_id = Fget (lface, Qface);
> + // FIXME why is (get 'tab-line 'face) 0?
A bug, I guess.
> + if (!FIXNATP (face_id))
> + // FIXME: I'm not sure what to do in this case
I'm not sure I understand why do you need to look at the existing
face's 'face' property? The original code didn't.
> DEFVAR_LISP ("face-new-frame-defaults", Vface_new_frame_defaults,
> doc: /* List of global face definitions (for internal use only.) */);
> - Vface_new_frame_defaults = Qnil;
> + Vface_new_frame_defaults =
> + make_hash_table (hashtest_eq, DEFAULT_HASH_SIZE, DEFAULT_REHASH_SIZE,
> + DEFAULT_REHASH_THRESHOLD, Qnil, Qnil);
Why do we need to start with a non-default hash-table?
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-12 4:30 bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined Clément Pit-Claudel
2020-05-12 6:42 ` martin rudalics
2020-05-12 15:27 ` Eli Zaretskii
@ 2020-05-15 14:03 ` Stefan Monnier
2020-05-15 14:34 ` Eli Zaretskii
2020-05-15 19:10 ` Clément Pit-Claudel
2021-04-06 6:35 ` Jashank Jeremy
3 siblings, 2 replies; 57+ messages in thread
From: Stefan Monnier @ 2020-05-15 14:03 UTC (permalink / raw)
To: Clément Pit-Claudel; +Cc: 41200
> Indeed, you're completely right; thanks! Replacing face_alist and
> Vface_new_frame_defaults with hash tables makes the worst example
> about 10 times faster, and with that change tooltips now take 30 to
> 50ms to display instead of 500-600ms in my real-life use case (my
> usual config). I have attached a patch.
Oh, this is great, makes a very noticeable difference.
I'm hoping it can make it into Emacs-27.1, tho it is probably rather
late for such a significant change.
> * I removed the function frame-face-alist and changed the type of the
> variable face-new-frame-defaults. Both were documented as internal.
> Should I add an ELisp implementation of frame-face-alist for
> compatibility? (It wouldn't be a perfect shim, since modifying its
> return value wouldn't do the same). For face-new-frame-defaults it's
> a bit trickier, since the variable now holds a hash table.
> Should I change its name to make the change obvious, at least?
The variable's name did not say "alist", so I don't see a need to change
it from that point of view. But I think it deserves a "--" since it's
supposed to be internal.
A quick grep revealed:
elpa/packages/context-coloring/fixtures/benchmark/faces.el: (mapcar #'car face-new-frame-defaults))
That doesn't seem very serious, but I haven't grep'd the MELPA packages.
I saw no such comparable use of `frame-face-alist` in the wild.
> * The name face_hash isn't ideal, since there's already a distinct
> notion of face hashes (hash codes). Can you think of a better name?
Yes: it's not a hash, it's a table.
I think the better names refer to the "conceptual" type rather than the
specific implementation type, so I prefer "map" or "table" to "hash",
"alist", "obarray", and whatnot.
> - command-execute 454 47%
[...]
> - face-set-after-frame-default 387 40%
> - face-spec-recalc 374 39%
> - make-face-x-resource-internal 296 30%
> - set-face-attributes-from-resources 273 28%
> - set-face-attribute-from-resource 219 22%
[...]
> - command-execute 768 80%
[...]
> - face-set-after-frame-default 674 70%
> - face-spec-recalc 660 69%
> - face-spec-set-2 350 36%
> - apply 348 36%
> - set-face-attribute 342 35%
> - internal-set-lisp-face-attribute 342 35%
> - frame-set-background-mode 331 34%
> - face-spec-recalc 284 29%
> - make-face-x-resource-internal 235 24%
Both of those profiles suggest that most of the time is still spent in
`face-spec-recalc`, so it would be worth trying harder to avoid calling
it or to speed it up somehow (presumably with some better memozing/caching).
The first profile also suggests that we spend too much time in
`make-face-x-resource-internal`.
What caught my eye in the second profile is the fact that we call
`face-spec-recalc` recursively. I suspect that recursion is not desired.
Stefan
PS: Maybe another way to speed this up is to do the `face-spec-recalc`
lazily (i.e. only when we encounter the face during redisplay), but
that's probably harder to do and not as good (since it will still be
slow in the case where the frame happens to display hundreds of faces).
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-15 14:03 ` Stefan Monnier
@ 2020-05-15 14:34 ` Eli Zaretskii
2020-05-15 19:10 ` Clément Pit-Claudel
1 sibling, 0 replies; 57+ messages in thread
From: Eli Zaretskii @ 2020-05-15 14:34 UTC (permalink / raw)
To: Stefan Monnier; +Cc: cpitclaudel, 41200
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>, 41200@debbugs.gnu.org
> Date: Fri, 15 May 2020 10:03:44 -0400
>
> I'm hoping it can make it into Emacs-27.1, tho it is probably rather
> late for such a significant change.
It's too late. Such a significant change on such a low level is got
to cause some issues, especially as parts of it are
backward-incompatible.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-15 11:05 ` Eli Zaretskii
@ 2020-05-15 14:59 ` Clément Pit-Claudel
2020-05-15 15:17 ` Eli Zaretskii
0 siblings, 1 reply; 57+ messages in thread
From: Clément Pit-Claudel @ 2020-05-15 14:59 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 41200
[-- Attachment #1: Type: text/plain, Size: 3577 bytes --]
Hi Eli,
Thanks a lot for the review. I have attached an updated version of the patch.
On 15/05/2020 07.05, Eli Zaretskii wrote:
> I'd like to keep the old face-new-frame-defaults and frame-face-alist
> for compatibility, but mention in the doc strings that they no longer
> return modifiable values, and perhaps deprecate them.
Done for frame-face-alist. But how can one do a read-only variable? Using the new variable watchers facility?
>> * The name face_hash isn't ideal, since there's already a distinct notion of face hashes (hash codes). Can you think of a better name?
>
> face_hash_table?
Thanks, good idea. I also liked Stefan's frame_face_map.
>> + // QUESTION: is this where this should be initialized?
>
> Yes, I think so. But do we need to do anything when frame is deleted
> as well?
I'm not sure (I don't know how garbage collection works on the C side in Elisp; I assumed the map would be collected).
>> -DEFUN ("frame-face-alist", Fframe_face_alist, Sframe_face_alist,
>> +// QUESTION: Should I add an ELisp version of frame-face-hash?
>
> You mean, frame-face-alist, right? Yes, most definitely: I imagine a
> lot of code out there uses that, and we wouldn't want to break that.
Done.
I looked around, but I didn't find many uses at all (for example, there are none in ELPA). I think this is likely because the function is documented as "For internal use only."
There are no uses of face-new-frame-defaults in ELPA either; online, I found many copies of lisp-mode and emacs-lisp-mode, which refer it, and a few functions derived from edebug-eval-defun, which references it.
> And I'm not sure we should have it only in Lisp: perhaps we should
> maintain the alist as well, and add/remove to/from it when a face is
> added or removed in the hash table. Otherwise this change of
> internals will have painful effect on packages that use the current
> APIs.
frame-face-alist is likely less crucial than face-new-frame-defaults, because it was already a function, so the return value has to be mutated in place to modify it (it couldn't be directly assigned). For both of these, however, how would we ensure that the alist remains in sync with the hashmap (that is, how do we catch modifications?)
>> + Lisp_Object lface = HASH_KEY(table, idx);
>> + Lisp_Object face_id = Fget (lface, Qface);
>> + // FIXME why is (get 'tab-line 'face) 0?
>
> A bug, I guess.
As far as I can see, these IDs are assigned by Finternal_make_lisp_face, and I *think* it is never called for tab-line? Should I make sure that it is? If so, where from?
>
>> + if (!FIXNATP (face_id))
>> + // FIXME: I'm not sure what to do in this case
>
> I'm not sure I understand why do you need to look at the existing
> face's 'face' property? The original code didn't.
The original code iterated over face-frame-alist in the order in which entries were added to it. If I understand correctly, iteration order isn't guaranteed on hash tables (is it?), so I had to find a different source of truth for these.
>
>> DEFVAR_LISP ("face-new-frame-defaults", Vface_new_frame_defaults,
>> doc: /* List of global face definitions (for internal use only.) */);
>> - Vface_new_frame_defaults = Qnil;
>> + Vface_new_frame_defaults =
>> + make_hash_table (hashtest_eq, DEFAULT_HASH_SIZE, DEFAULT_REHASH_SIZE,
>> + DEFAULT_REHASH_THRESHOLD, Qnil, Qnil);
>
> Why do we need to start with a non-default hash-table?
I wanted to use `eq' instead of `eql' as the test (is that what you were asking?)
[-- Attachment #2: 0001-Store-frame-faces-in-hash-tables-instead-of-alists.patch --]
[-- Type: text/x-patch, Size: 13026 bytes --]
From 86ea2f96e4e3a2161f68f13b251620b98489fd35 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Pit-Claudel?= <clement.pitclaudel@live.com>
Date: Tue, 12 May 2020 21:48:32 -0400
Subject: [PATCH] Store frame faces in hash tables instead of alists
* lisp/emacs-lisp/edebug.el (edebug-eval-defun):
* lisp/progmodes/elisp-mode.el (elisp--eval-defun-1):
* lisp/frame.el (frame-set-background-mode):
* lisp/faces.el (face-list): Update to work with hash
tables instead of alists.
(frame-face-alist): Reimplement using frame-face-map.
* src/frame.h (struct frame): Remove face_alist, add face_map.
(fset_face_alist): Remove.
(fset_face_map): New function.
* src/frame.c (make_frame): Initialize f->face_map.
(Fmake_terminal_frame): Update to work with hash tables instead of
alists.
* src/xfaces.c (lface_from_face_name_no_resolve):
(Finternal_make_lisp_face):
(update_face_from_frame_parameter): Update to work with hash tables
instead of alists.
* src/xfaces.c (Fframe_face_map): New function.
(Fframe_face_alist): Move to face.el.
(init_xfaces): Compute face IDs from they face property, not from
their position in face_alist.
(syms_of_xfaces): Remove frame_face_alist, add frame_face_map;
change Vface_new_frame_defaults into a hash table.
---
lisp/emacs-lisp/edebug.el | 3 +-
lisp/faces.el | 13 +++++++-
lisp/frame.el | 2 +-
lisp/progmodes/elisp-mode.el | 3 +-
src/frame.c | 20 +++++++----
src/frame.h | 8 ++---
src/xfaces.c | 65 +++++++++++++++++++-----------------
7 files changed, 67 insertions(+), 47 deletions(-)
diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index 78461185d3..97ca964eef 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -488,8 +488,7 @@ edebug-eval-defun
(set-default (nth 1 form) (eval (nth 2 form) lexical-binding)))
((eq (car form) 'defface)
;; Reset the face.
- (setq face-new-frame-defaults
- (assq-delete-all (nth 1 form) face-new-frame-defaults))
+ (remhash (nth 1 form) face-new-frame-defaults)
(put (nth 1 form) 'face-defface-spec nil)
(put (nth 1 form) 'face-documentation (nth 3 form))
;; See comments in `eval-defun-1' for purpose of code below
diff --git a/lisp/faces.el b/lisp/faces.el
index e707f6f4b6..c45921bb48 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -176,10 +176,21 @@ face-font-registry-alternatives
;;; Creation, copying.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+(defun frame-face-alist (&optional frame)
+ "Return an alist of frame-local faces defined on FRAME.
+This alist is a copy of the contents of `frame-face-map'.
+For internal use only."
+ (declare (obsolete frame-face-map "28.1"))
+ (let ((faces))
+ (maphash (lambda (face spec) (push (cons face spec) faces))
+ (frame-face-map frame))
+ faces))
(defun face-list ()
"Return a list of all defined faces."
- (mapcar #'car face-new-frame-defaults))
+ (let ((faces nil))
+ (maphash (lambda (face _) (push face faces)) face-new-frame-defaults)
+ faces))
(defun make-face (face)
"Define a new face with name FACE, a symbol.
diff --git a/lisp/frame.el b/lisp/frame.el
index 6c2f774709..14c9112aad 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -1227,7 +1227,7 @@ frame-set-background-mode
;; during startup with -rv on the command
;; line for the initial frame, because frames
;; are not recorded in the pdump file.
- (assq face (frame-face-alist))
+ (gethash face (frame-face-map))
(face-spec-match-p face
(face-user-default-spec face)
;; FIXME: why selected-frame and
diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index b737134f90..dc8688c864 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -1304,8 +1304,7 @@ elisp--eval-defun-1
((eq (car form) 'custom-declare-face)
;; Reset the face.
(let ((face-symbol (eval (nth 1 form) lexical-binding)))
- (setq face-new-frame-defaults
- (assq-delete-all face-symbol face-new-frame-defaults))
+ (remhash face-new-frame-defaults face-symbol)
(put face-symbol 'face-defface-spec nil)
(put face-symbol 'face-override-spec nil))
form)
diff --git a/src/frame.c b/src/frame.c
index c871e4fd99..514b390910 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -946,6 +946,10 @@ make_frame (bool mini_p)
rw->total_lines = mini_p ? 9 : 10;
rw->pixel_height = rw->total_lines * FRAME_LINE_HEIGHT (f);
+ fset_face_map
+ (f, make_hash_table (hashtest_eq, DEFAULT_HASH_SIZE, DEFAULT_REHASH_SIZE,
+ DEFAULT_REHASH_THRESHOLD, Qnil, false));
+
if (mini_p)
{
mw->top_line = rw->total_lines;
@@ -1254,7 +1258,7 @@ DEFUN ("make-terminal-frame", Fmake_terminal_frame, Smake_terminal_frame,
{
struct frame *f;
struct terminal *t = NULL;
- Lisp_Object frame, tem;
+ Lisp_Object frame;
struct frame *sf = SELECTED_FRAME ();
#ifdef MSDOS
@@ -1336,14 +1340,16 @@ DEFUN ("make-terminal-frame", Fmake_terminal_frame, Smake_terminal_frame,
store_in_alist (&parms, Qminibuffer, Qt);
Fmodify_frame_parameters (frame, parms);
- /* Make the frame face alist be frame-specific, so that each
+ /* Make the frame face hash be frame-specific, so that each
frame could change its face definitions independently. */
- fset_face_alist (f, Fcopy_alist (sf->face_alist));
- /* Simple Fcopy_alist isn't enough, because we need the contents of
- the vectors which are the CDRs of associations in face_alist to
+ fset_face_map (f, Fcopy_hash_table (sf->face_map));
+ /* Simple copy_hash_table isn't enough, because we need the contents of
+ the vectors which are the values in face_map to
be copied as well. */
- for (tem = f->face_alist; CONSP (tem); tem = XCDR (tem))
- XSETCDR (XCAR (tem), Fcopy_sequence (XCDR (XCAR (tem))));
+ ptrdiff_t idx = 0;
+ struct Lisp_Hash_Table *table = XHASH_TABLE (f->face_map);
+ for (idx = 0; idx < table->count; ++idx)
+ set_hash_value_slot (table, idx, Fcopy_sequence (HASH_VALUE (table, idx)));
f->can_set_window_size = true;
f->after_make_frame = true;
diff --git a/src/frame.h b/src/frame.h
index 476bac67fa..f8915b3c1a 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -158,8 +158,8 @@ #define EMACS_FRAME_H
There are four additional elements of nil at the end, to terminate. */
Lisp_Object menu_bar_items;
- /* Alist of elements (FACE-NAME . FACE-VECTOR-DATA). */
- Lisp_Object face_alist;
+ /* Hash table of FACE-NAME keys and FACE-VECTOR-DATA values. */
+ Lisp_Object face_map;
/* A vector that records the entire structure of this frame's menu bar.
For the format of the data, see extensive comments in xmenu.c.
@@ -661,9 +661,9 @@ fset_condemned_scroll_bars (struct frame *f, Lisp_Object val)
f->condemned_scroll_bars = val;
}
INLINE void
-fset_face_alist (struct frame *f, Lisp_Object val)
+fset_face_map (struct frame *f, Lisp_Object val)
{
- f->face_alist = val;
+ f->face_map = val;
}
#if defined (HAVE_WINDOW_SYSTEM)
INLINE void
diff --git a/src/xfaces.c b/src/xfaces.c
index 7d7aff95c1..89e9a10a0a 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -1843,13 +1843,11 @@ lface_from_face_name_no_resolve (struct frame *f, Lisp_Object face_name,
Lisp_Object lface;
if (f)
- lface = assq_no_quit (face_name, f->face_alist);
+ lface = Fgethash (face_name, f->face_map, Qnil);
else
- lface = assq_no_quit (face_name, Vface_new_frame_defaults);
+ lface = Fgethash (face_name, Vface_new_frame_defaults, Qnil);
- if (CONSP (lface))
- lface = XCDR (lface);
- else if (signal_p)
+ if (signal_p && NILP (lface))
signal_error ("Invalid face", face_name);
check_lface (lface);
@@ -2736,8 +2734,7 @@ DEFUN ("internal-make-lisp-face", Finternal_make_lisp_face,
{
global_lface = make_vector (LFACE_VECTOR_SIZE, Qunspecified);
ASET (global_lface, 0, Qface);
- Vface_new_frame_defaults = Fcons (Fcons (face, global_lface),
- Vface_new_frame_defaults);
+ Fputhash (face, global_lface, Vface_new_frame_defaults);
/* Assign the new Lisp face a unique ID. The mapping from Lisp
face id to Lisp face is given by the vector lface_id_to_name.
@@ -2763,7 +2760,7 @@ DEFUN ("internal-make-lisp-face", Finternal_make_lisp_face,
{
lface = make_vector (LFACE_VECTOR_SIZE, Qunspecified);
ASET (lface, 0, Qface);
- fset_face_alist (f, Fcons (Fcons (face, lface), f->face_alist));
+ Fputhash (face, lface, f->face_map);
}
else
for (i = 1; i < LFACE_VECTOR_SIZE; ++i)
@@ -3508,7 +3505,7 @@ update_face_from_frame_parameter (struct frame *f, Lisp_Object param,
/* If there are no faces yet, give up. This is the case when called
from Fx_create_frame, and we do the necessary things later in
face-set-after-frame-defaults. */
- if (NILP (f->face_alist))
+ if (XFIXNAT (Fhash_table_count (f->face_map)) == 0)
return;
if (EQ (param, Qforeground_color))
@@ -4174,14 +4171,13 @@ DEFUN ("internal-lisp-face-empty-p", Finternal_lisp_face_empty_p,
return i == LFACE_VECTOR_SIZE ? Qt : Qnil;
}
-
-DEFUN ("frame-face-alist", Fframe_face_alist, Sframe_face_alist,
+DEFUN ("frame-face-map", Fframe_face_map, Sframe_face_map,
0, 1, 0,
- doc: /* Return an alist of frame-local faces defined on FRAME.
+ doc: /* Return a hash table of frame-local faces defined on FRAME.
For internal use only. */)
(Lisp_Object frame)
{
- return decode_live_frame (frame)->face_alist;
+ return decode_live_frame (frame)->face_map;
}
@@ -6678,30 +6674,37 @@ DEFUN ("show-face-resources", Fshow_face_resources, Sshow_face_resources,
#ifdef HAVE_PDUMPER
/* All the faces defined during loadup are recorded in
- face-new-frame-defaults, with the last face first in the list. We
- need to set next_lface_id to the next face ID number, so that any
- new faces defined in this session will have face IDs different from
- those defined during loadup. We also need to set up the
- lface_id_to_name[] array for the faces that were defined during
- loadup. */
+ face-new-frame-defaults. We need to set next_lface_id to the next
+ face ID number, so that any new faces defined in this session will
+ have face IDs different from those defined during loadup. We also
+ need to set up the lface_id_to_name[] array for the faces that were
+ defined during loadup. */
void
init_xfaces (void)
{
- if (CONSP (Vface_new_frame_defaults))
+ int nfaces = XFIXNAT (Fhash_table_count (Vface_new_frame_defaults));
+ if (nfaces > 0)
{
/* Allocate the lface_id_to_name[] array. */
- lface_id_to_name_size = next_lface_id =
- XFIXNAT (Flength (Vface_new_frame_defaults));
+ lface_id_to_name_size = next_lface_id = nfaces;
lface_id_to_name = xnmalloc (next_lface_id, sizeof *lface_id_to_name);
/* Store the faces. */
- Lisp_Object tail;
- int i = next_lface_id - 1;
- for (tail = Vface_new_frame_defaults; CONSP (tail); tail = XCDR (tail))
+ struct Lisp_Hash_Table* table = XHASH_TABLE (Vface_new_frame_defaults);
+ for (ptrdiff_t idx = 0; idx < nfaces; ++idx)
{
- Lisp_Object lface = XCAR (tail);
- eassert (i >= 0);
- lface_id_to_name[i--] = XCAR (lface);
+ Lisp_Object lface = HASH_KEY (table, idx);
+ Lisp_Object face_id = Fget (lface, Qface);
+ // FIXME why is (get 'tab-line 'face) 0?
+ if (!FIXNATP (face_id))
+ // FIXME: I'm not sure what to do in this case
+ printf ("Face %s has no id\n", SDATA (SYMBOL_NAME (lface)));
+ else
+ {
+ int id = XFIXNAT (face_id);
+ eassert (id >= 0);
+ lface_id_to_name[id] = lface;
+ }
}
}
face_attr_sym[0] = Qface;
@@ -6855,7 +6858,7 @@ syms_of_xfaces (void)
defsubr (&Sinternal_copy_lisp_face);
defsubr (&Sinternal_merge_in_global_face);
defsubr (&Sface_font);
- defsubr (&Sframe_face_alist);
+ defsubr (&Sframe_face_map);
defsubr (&Sdisplay_supports_face_attributes_p);
defsubr (&Scolor_distance);
defsubr (&Sinternal_set_font_selection_order);
@@ -6881,7 +6884,9 @@ syms_of_xfaces (void)
DEFVAR_LISP ("face-new-frame-defaults", Vface_new_frame_defaults,
doc: /* List of global face definitions (for internal use only.) */);
- Vface_new_frame_defaults = Qnil;
+ Vface_new_frame_defaults =
+ make_hash_table (hashtest_eq, DEFAULT_HASH_SIZE, DEFAULT_REHASH_SIZE,
+ DEFAULT_REHASH_THRESHOLD, Qnil, false);
DEFVAR_LISP ("face-default-stipple", Vface_default_stipple,
doc: /* Default stipple pattern used on monochrome displays.
--
2.17.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-15 14:59 ` Clément Pit-Claudel
@ 2020-05-15 15:17 ` Eli Zaretskii
2020-05-15 15:33 ` Noam Postavsky
2020-05-15 16:22 ` Clément Pit-Claudel
0 siblings, 2 replies; 57+ messages in thread
From: Eli Zaretskii @ 2020-05-15 15:17 UTC (permalink / raw)
To: Clément Pit-Claudel; +Cc: 41200
> Cc: 41200@debbugs.gnu.org
> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Fri, 15 May 2020 10:59:36 -0400
>
> > I'd like to keep the old face-new-frame-defaults and frame-face-alist
> > for compatibility, but mention in the doc strings that they no longer
> > return modifiable values, and perhaps deprecate them.
>
> Done for frame-face-alist. But how can one do a read-only variable? Using the new variable watchers facility?
At the very least mention in the doc string. I don't think using
watchable symbols is needed here, it sounds gross. If there's an
outcry that this breaks too much code out there, we could revisit
this.
> >> * The name face_hash isn't ideal, since there's already a distinct notion of face hashes (hash codes). Can you think of a better name?
> >
> > face_hash_table?
>
> Thanks, good idea. I also liked Stefan's frame_face_map.
"Map" is too general, IMO, it doesn't tell enough about the kind of
object it is.
> >> + // QUESTION: is this where this should be initialized?
> >
> > Yes, I think so. But do we need to do anything when frame is deleted
> > as well?
>
> I'm not sure (I don't know how garbage collection works on the C side in Elisp; I assumed the map would be collected).
I guess so.
> > You mean, frame-face-alist, right? Yes, most definitely: I imagine a
> > lot of code out there uses that, and we wouldn't want to break that.
>
> Done.
>
> I looked around, but I didn't find many uses at all (for example, there are none in ELPA). I think this is likely because the function is documented as "For internal use only."
>
> There are no uses of face-new-frame-defaults in ELPA either; online, I found many copies of lisp-mode and emacs-lisp-mode, which refer it, and a few functions derived from edebug-eval-defun, which references it.
That means we will probably be able to remove it earlier than I
feared.
> > And I'm not sure we should have it only in Lisp: perhaps we should
> > maintain the alist as well, and add/remove to/from it when a face is
> > added or removed in the hash table. Otherwise this change of
> > internals will have painful effect on packages that use the current
> > APIs.
>
> frame-face-alist is likely less crucial than face-new-frame-defaults, because it was already a function, so the return value has to be mutated in place to modify it (it couldn't be directly assigned). For both of these, however, how would we ensure that the alist remains in sync with the hashmap (that is, how do we catch modifications?)
I thought about updating the alist when the hash-table is modified.
Since you always know whether the face is already in the hash-table,
you don't need to scan the alist looking for it.
I'd really like to know that no one is using these before we make the
final decision about this.
> >> + Lisp_Object lface = HASH_KEY(table, idx);
> >> + Lisp_Object face_id = Fget (lface, Qface);
> >> + // FIXME why is (get 'tab-line 'face) 0?
> >
> > A bug, I guess.
>
> As far as I can see, these IDs are assigned by Finternal_make_lisp_face, and I *think* it is never called for tab-line?
Most probably.
> Should I make sure that it is?
Yes, I think so.
> If so, where from?
I think the problem is that tab-line is declared a basic face, but its
defface form is not in faces.el.
> >> + if (!FIXNATP (face_id))
> >> + // FIXME: I'm not sure what to do in this case
> >
> > I'm not sure I understand why do you need to look at the existing
> > face's 'face' property? The original code didn't.
>
> The original code iterated over face-frame-alist in the order in which entries were added to it. If I understand correctly, iteration order isn't guaranteed on hash tables (is it?), so I had to find a different source of truth for these.
Maybe we should store the ID with the face? I think we wanted to get
rid of the 'face' property of the faces at some point.
> >> DEFVAR_LISP ("face-new-frame-defaults", Vface_new_frame_defaults,
> >> doc: /* List of global face definitions (for internal use only.) */);
> >> - Vface_new_frame_defaults = Qnil;
> >> + Vface_new_frame_defaults =
> >> + make_hash_table (hashtest_eq, DEFAULT_HASH_SIZE, DEFAULT_REHASH_SIZE,
> >> + DEFAULT_REHASH_THRESHOLD, Qnil, Qnil);
> >
> > Why do we need to start with a non-default hash-table?
>
> I wanted to use `eq' instead of `eql' as the test (is that what you were asking?)
No, I was asking why not start with it as nil and actually make a
hash-table when we first need it.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-15 15:17 ` Eli Zaretskii
@ 2020-05-15 15:33 ` Noam Postavsky
2020-05-15 16:22 ` Clément Pit-Claudel
1 sibling, 0 replies; 57+ messages in thread
From: Noam Postavsky @ 2020-05-15 15:33 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Clément Pit-Claudel, 41200
Eli Zaretskii <eliz@gnu.org> writes:
>> Done for frame-face-alist. But how can one do a read-only variable?
>> Using the new variable watchers facility?
>
> At the very least mention in the doc string. I don't think using
> watchable symbols is needed here, it sounds gross. If there's an
> outcry that this breaks too much code out there, we could revisit
> this.
You can use make_symbol_constant to make a variable read-only. You
could also simulate it with a variable watcher that signals an error,
but there's no sense in doing that when make_symbol_constant exists.
Neither of these makes the list object itself read-only though.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-15 15:17 ` Eli Zaretskii
2020-05-15 15:33 ` Noam Postavsky
@ 2020-05-15 16:22 ` Clément Pit-Claudel
2020-05-15 17:28 ` Eli Zaretskii
1 sibling, 1 reply; 57+ messages in thread
From: Clément Pit-Claudel @ 2020-05-15 16:22 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 41200, Anders Lindgren
On 15/05/2020 11.17, Eli Zaretskii wrote:
>>>> * The name face_hash isn't ideal, since there's already a distinct notion of face hashes (hash codes). Can you think of a better name?
>>>
>>> face_hash_table?
>>
>> Thanks, good idea. I also liked Stefan's frame_face_map.
>
> "Map" is too general, IMO, it doesn't tell enough about the kind of
> object it is.
Got it. Is face_table better? (that was Stefan's other suggestion)
>>> And I'm not sure we should have it only in Lisp: perhaps we should
>>> maintain the alist as well, and add/remove to/from it when a face is
>>> added or removed in the hash table. Otherwise this change of
>>> internals will have painful effect on packages that use the current
>>> APIs.
>>
>> frame-face-alist is likely less crucial than face-new-frame-defaults, because it was already a function, so the return value has to be mutated in place to modify it (it couldn't be directly assigned). For both of these, however, how would we ensure that the alist remains in sync with the hashmap (that is, how do we catch modifications?)
>
> I thought about updating the alist when the hash-table is modified.
> Since you always know whether the face is already in the hash-table,
> you don't need to scan the alist looking for it.
Would that be done in C, or in any place where frame-new-face-defaults is modified? (for example, edebug changes face-new-frame-defaults in one place; if we keep the alist in addition to the hash table, would it modify both or would there be a C mechanism that mirrors hash-table modifications to the alist?)
> I'd really like to know that no one is using these before we make the
> final decision about this.
That's a fair point. While I couldn't find uses of frame-face-alist, there are a few uses of face-new-frame-defaults-alist: https://github.com/pestctrl/emacs-config/commit/31d6bff97dacf60f71066902a23d37e59b4c1288 is from someone who uses that to speed up temporary frame creation(!), https://github.com/Lindydancer/face-explorer/blob/13bd4553bc4b09215a04d0267be1cb4ed834775c/test/face-explorer-test-faces.el is from Anders Lindgren (in CC; hi Andres! We're considering replacing face-new-frame-defaults-alist with a hash table, would that be an issue?), who uses it to remove a face, and https://github.com/Wilfred/elisp-fu/blob/92c491584f466ee729ea1b328234636e65e2c665/elisp-fu.el includes code that's the same as in eval-defun.
I have a variant of the patch that keeps the alist variable, but I fear that it's worse than removing it: since changes to the alist won't be propagated to the hash table, it might be better to error out with a compilation error?
Btw, I have one more question: some callers of face-list seems to rely on the order of faces added to it, so it would be better to preserve that order. Since hash-tables are not necessarily ordered, should I sort faces by face-id before returning them?
>> If so, where from?
>
> I think the problem is that tab-line is declared a basic face, but its
> defface form is not in faces.el.
Ah, good catch. Current there's a defface for tab-bar in lisp/tab-bar, and since that's preloaded it works, but the defface for tab-line is in lisp/tab-line.el and so isn't preloaded.
Should I move both to faces.el?
>>>> + if (!FIXNATP (face_id))
>>>> + // FIXME: I'm not sure what to do in this case
>>>
>>> I'm not sure I understand why do you need to look at the existing
>>> face's 'face' property? The original code didn't.
>>
>> The original code iterated over face-frame-alist in the order in which entries were added to it. If I understand correctly, iteration order isn't guaranteed on hash tables (is it?), so I had to find a different source of truth for these.
>
> Maybe we should store the ID with the face? I think we wanted to get
> rid of the 'face' property of the faces at some point.
Sounds reasonable; but where would we store it? Right now faces are just symbols, right?
> No, I was asking why not start with it as nil and actually make a
> hash-table when we first need it.
Oh, I thought it would be simpler to always have a hash table instead of having to sanity check every time.
Thanks a lot,
Clément.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-15 16:22 ` Clément Pit-Claudel
@ 2020-05-15 17:28 ` Eli Zaretskii
2020-05-15 18:50 ` Clément Pit-Claudel
0 siblings, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2020-05-15 17:28 UTC (permalink / raw)
To: Clément Pit-Claudel; +Cc: 41200, andlind
> Cc: 41200@debbugs.gnu.org, Anders Lindgren <andlind@gmail.com>
> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Fri, 15 May 2020 12:22:53 -0400
>
> >>> face_hash_table?
> >>
> >> Thanks, good idea. I also liked Stefan's frame_face_map.
> >
> > "Map" is too general, IMO, it doesn't tell enough about the kind of
> > object it is.
>
> Got it. Is face_table better? (that was Stefan's other suggestion)
Is anything wrong with face_hash_table?
> > I thought about updating the alist when the hash-table is modified.
> > Since you always know whether the face is already in the hash-table,
> > you don't need to scan the alist looking for it.
>
> Would that be done in C, or in any place where frame-new-face-defaults is modified? (for example, edebug changes face-new-frame-defaults in one place; if we keep the alist in addition to the hash table, would it modify both or would there be a C mechanism that mirrors hash-table modifications to the alist?)
The latter was what I had in mind.
> I have a variant of the patch that keeps the alist variable, but I fear that it's worse than removing it: since changes to the alist won't be propagated to the hash table, it might be better to error out with a compilation error?
Not sure yet, it depends on how this is used.
> Btw, I have one more question: some callers of face-list seems to rely on the order of faces added to it, so it would be better to preserve that order. Since hash-tables are not necessarily ordered, should I sort faces by face-id before returning them?
Yes, I think so.
> > I think the problem is that tab-line is declared a basic face, but its
> > defface form is not in faces.el.
>
> Ah, good catch. Current there's a defface for tab-bar in lisp/tab-bar, and since that's preloaded it works, but the defface for tab-line is in lisp/tab-line.el and so isn't preloaded.
> Should I move both to faces.el?
Yes, I think so.
> >>> I'm not sure I understand why do you need to look at the existing
> >>> face's 'face' property? The original code didn't.
> >>
> >> The original code iterated over face-frame-alist in the order in which entries were added to it. If I understand correctly, iteration order isn't guaranteed on hash tables (is it?), so I had to find a different source of truth for these.
> >
> > Maybe we should store the ID with the face? I think we wanted to get
> > rid of the 'face' property of the faces at some point.
>
> Sounds reasonable; but where would we store it? Right now faces are just symbols, right?
Don't hash-tables allow us to store more than one item in each slot?
> > No, I was asking why not start with it as nil and actually make a
> > hash-table when we first need it.
>
> Oh, I thought it would be simpler to always have a hash table instead of having to sanity check every time.
What is the default size of a hash-table?
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-15 17:28 ` Eli Zaretskii
@ 2020-05-15 18:50 ` Clément Pit-Claudel
2020-05-15 19:05 ` Eli Zaretskii
2020-05-16 23:03 ` Juri Linkov
0 siblings, 2 replies; 57+ messages in thread
From: Clément Pit-Claudel @ 2020-05-15 18:50 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 41200, Juri Linkov
[-- Attachment #1: Type: text/plain, Size: 2639 bytes --]
On 15/05/2020 13.28, Eli Zaretskii wrote:
>> Cc: 41200@debbugs.gnu.org, Anders Lindgren <andlind@gmail.com>
>> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
>> Date: Fri, 15 May 2020 12:22:53 -0400
>>
>>>>> face_hash_table?
>>>>
>>>> Thanks, good idea. I also liked Stefan's frame_face_map.
>>>
>>> "Map" is too general, IMO, it doesn't tell enough about the kind of
>>> object it is.
>>
>> Got it. Is face_table better? (that was Stefan's other suggestion)
>
> Is anything wrong with face_hash_table?
Nope; I've attached a new patch.
>> Btw, I have one more question: some callers of face-list seems to rely on the order of faces added to it, so it would be better to preserve that order. Since hash-tables are not necessarily ordered, should I sort faces by face-id before returning them?
Done in the attached patch as well.
>>> I think the problem is that tab-line is declared a basic face, but its
>>> defface form is not in faces.el.
>>
>> Ah, good catch. Current there's a defface for tab-bar in lisp/tab-bar, and since that's preloaded it works, but the defface for tab-line is in lisp/tab-line.el and so isn't preloaded.
>> Should I move both to faces.el?
>>
> Yes, I think so.
Thanks. I will ask Juri to confirm before moving them, because I realize now that they have a custom group.
Juri (CC'd; hi Juri!), was there a reason to make tab-bar and tab-line basic faces? I see they are both in their own files and groups, instead of being in faces.el.
>>>>> I'm not sure I understand why do you need to look at the existing
>>>>> face's 'face' property? The original code didn't.
>>>>
>>>> The original code iterated over face-frame-alist in the order in which entries were added to it. If I understand correctly, iteration order isn't guaranteed on hash tables (is it?), so I had to find a different source of truth for these.
>>>
>>> Maybe we should store the ID with the face? I think we wanted to get
>>> rid of the 'face' property of the faces at some point.
>>
>> Sounds reasonable; but where would we store it? Right now faces are just symbols, right?
>
> Don't hash-tables allow us to store more than one item in each slot?
They do, so I can certainly store (face-id . face-vector) in the hash table. Done in the attached patch. I only do this in Vface_new_frame_defaults, not in frame->face_hash_table.
>>> No, I was asking why not start with it as nil and actually make a
>>> hash-table when we first need it.
>>
>> Oh, I thought it would be simpler to always have a hash table instead of having to sanity check every time.
>
> What is the default size of a hash-table?
65 entries, I think.
[-- Attachment #2: 0001-Store-frame-faces-in-hash-tables-instead-of-alists.patch --]
[-- Type: text/x-patch, Size: 16886 bytes --]
From be915d6d2ea7a739026dc4a53fab64f78efab2bd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Pit-Claudel?= <clement.pitclaudel@live.com>
Date: Tue, 12 May 2020 21:48:32 -0400
Subject: [PATCH] Store frame faces in hash tables instead of alists
* src/frame.h (struct frame): Add face_hash_table, remove face_alist.
(fset_face_hash_table): New function.
(fset_face_alist): Remove.
* src/frame.c (make_frame): Initialize f->face_hash_table.
(Fmake_terminal_frame): Update to work with hash tables instead of
alists.
* src/xfaces.c (lface_from_face_name_no_resolve):
(Finternal_make_lisp_face):
(update_face_from_frame_parameter): Update to work with hash tables
instead of alists.
(Fframe_face_hash_table): New function.
(Fframe_face_alist): Move to faces.el as frame-face-alist.
(syms_of_xfaces): Add frame_face_hash_table.
* lisp/emacs-lisp/edebug.el (edebug-eval-defun):
* lisp/progmodes/elisp-mode.el (elisp--eval-defun-1):
* lisp/frame.el (frame-set-background-mode): Update to work with hash
tables instead of alists.
* lisp/faces.el (face-new-frame-defaults): Mark obsolete.
(face-list): Update to use face--new-frame-defaults.
(frame-face-alist): Moved here from src/xfaces.c.
---
lisp/custom.el | 2 +-
lisp/emacs-lisp/edebug.el | 3 +-
lisp/faces.el | 23 ++++++++--
lisp/frame.el | 2 +-
lisp/progmodes/elisp-mode.el | 3 +-
src/frame.c | 20 ++++++---
src/frame.h | 8 ++--
src/xfaces.c | 82 ++++++++++++++++++------------------
8 files changed, 83 insertions(+), 60 deletions(-)
diff --git a/lisp/custom.el b/lisp/custom.el
index 885c486c5e..3ac8bac40f 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -898,7 +898,7 @@ custom-push-theme
;; the value to a fake theme, `changed'. If the theme is
;; later disabled, we use this to bring back the old value.
;;
- ;; For faces, we just use `face-new-frame-defaults' to
+ ;; For faces, we just use `face--new-frame-defaults' to
;; recompute when the theme is disabled.
(when (and (eq prop 'theme-value)
(boundp symbol))
diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index 78461185d3..76a75c04df 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -488,8 +488,7 @@ edebug-eval-defun
(set-default (nth 1 form) (eval (nth 2 form) lexical-binding)))
((eq (car form) 'defface)
;; Reset the face.
- (setq face-new-frame-defaults
- (assq-delete-all (nth 1 form) face-new-frame-defaults))
+ (remhash (nth 1 form) face--new-frame-defaults)
(put (nth 1 form) 'face-defface-spec nil)
(put (nth 1 form) 'face-documentation (nth 3 form))
;; See comments in `eval-defun-1' for purpose of code below
diff --git a/lisp/faces.el b/lisp/faces.el
index e707f6f4b6..bb51797a38 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -176,10 +176,27 @@ face-font-registry-alternatives
;;; Creation, copying.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+(make-obsolete-variable
+ 'face-new-frame-defaults
+ "use `face--new-frame-defaults' or `face-alist' instead."
+ "28.1")
+
+(defun frame-face-alist (&optional frame)
+ "Return an alist of frame-local faces defined on FRAME.
+This alist is a copy of the contents of `frame--face-hash-table'.
+For internal use only."
+ (declare (obsolete frame--face-hash-table "28.1"))
+ (let ((faces))
+ (maphash (lambda (face spec) (push `(,(car spec) ,face . ,(cdr spec)) faces))
+ (frame--face-hash-table frame))
+ (mapcar #'cdr (sort faces (lambda (f1 f2) (< (car f1) (car f2)))))))
(defun face-list ()
"Return a list of all defined faces."
- (mapcar #'car face-new-frame-defaults))
+ (let ((faces))
+ (maphash (lambda (face spec) (push `(,(car spec) . ,face) faces))
+ face--new-frame-defaults)
+ (mapcar #'cdr (sort faces (lambda (f1 f2) (< (car f1) (car f2)))))))
(defun make-face (face)
"Define a new face with name FACE, a symbol.
@@ -2097,7 +2114,7 @@ x-create-frame-with-faces
(defun face-set-after-frame-default (frame &optional parameters)
"Initialize the frame-local faces of FRAME.
Calculate the face definitions using the face specs, custom theme
-settings, X resources, and `face-new-frame-defaults'.
+settings, X resources, and `face--new-frame-defaults'.
Finally, apply any relevant face attributes found amongst the
frame parameters in PARAMETERS."
;; The `reverse' is so that `default' goes first.
@@ -2106,7 +2123,7 @@ face-set-after-frame-default
(progn
;; Initialize faces from face spec and custom theme.
(face-spec-recalc face frame)
- ;; Apply attributes specified by face-new-frame-defaults
+ ;; Apply attributes specified by face--new-frame-defaults
(internal-merge-in-global-face face frame))
;; Don't let invalid specs prevent frame creation.
(error nil)))
diff --git a/lisp/frame.el b/lisp/frame.el
index 6c2f774709..66b5c6aa8d 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -1227,7 +1227,7 @@ frame-set-background-mode
;; during startup with -rv on the command
;; line for the initial frame, because frames
;; are not recorded in the pdump file.
- (assq face (frame-face-alist))
+ (gethash face (frame--face-hash-table))
(face-spec-match-p face
(face-user-default-spec face)
;; FIXME: why selected-frame and
diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index b737134f90..75054b8818 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -1304,8 +1304,7 @@ elisp--eval-defun-1
((eq (car form) 'custom-declare-face)
;; Reset the face.
(let ((face-symbol (eval (nth 1 form) lexical-binding)))
- (setq face-new-frame-defaults
- (assq-delete-all face-symbol face-new-frame-defaults))
+ (remhash face--new-frame-defaults face-symbol)
(put face-symbol 'face-defface-spec nil)
(put face-symbol 'face-override-spec nil))
form)
diff --git a/src/frame.c b/src/frame.c
index c871e4fd99..0843d53559 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -946,6 +946,10 @@ make_frame (bool mini_p)
rw->total_lines = mini_p ? 9 : 10;
rw->pixel_height = rw->total_lines * FRAME_LINE_HEIGHT (f);
+ fset_face_hash_table
+ (f, make_hash_table (hashtest_eq, DEFAULT_HASH_SIZE, DEFAULT_REHASH_SIZE,
+ DEFAULT_REHASH_THRESHOLD, Qnil, false));
+
if (mini_p)
{
mw->top_line = rw->total_lines;
@@ -1254,7 +1258,7 @@ DEFUN ("make-terminal-frame", Fmake_terminal_frame, Smake_terminal_frame,
{
struct frame *f;
struct terminal *t = NULL;
- Lisp_Object frame, tem;
+ Lisp_Object frame;
struct frame *sf = SELECTED_FRAME ();
#ifdef MSDOS
@@ -1336,14 +1340,16 @@ DEFUN ("make-terminal-frame", Fmake_terminal_frame, Smake_terminal_frame,
store_in_alist (&parms, Qminibuffer, Qt);
Fmodify_frame_parameters (frame, parms);
- /* Make the frame face alist be frame-specific, so that each
+ /* Make the frame face hash be frame-specific, so that each
frame could change its face definitions independently. */
- fset_face_alist (f, Fcopy_alist (sf->face_alist));
- /* Simple Fcopy_alist isn't enough, because we need the contents of
- the vectors which are the CDRs of associations in face_alist to
+ fset_face_hash_table (f, Fcopy_hash_table (sf->face_hash_table));
+ /* Simple copy_hash_table isn't enough, because we need the contents of
+ the vectors which are the values in face_hash_table to
be copied as well. */
- for (tem = f->face_alist; CONSP (tem); tem = XCDR (tem))
- XSETCDR (XCAR (tem), Fcopy_sequence (XCDR (XCAR (tem))));
+ ptrdiff_t idx = 0;
+ struct Lisp_Hash_Table *table = XHASH_TABLE (f->face_hash_table);
+ for (idx = 0; idx < table->count; ++idx)
+ set_hash_value_slot (table, idx, Fcopy_sequence (HASH_VALUE (table, idx)));
f->can_set_window_size = true;
f->after_make_frame = true;
diff --git a/src/frame.h b/src/frame.h
index 476bac67fa..9761253085 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -158,8 +158,8 @@ #define EMACS_FRAME_H
There are four additional elements of nil at the end, to terminate. */
Lisp_Object menu_bar_items;
- /* Alist of elements (FACE-NAME . FACE-VECTOR-DATA). */
- Lisp_Object face_alist;
+ /* Hash table of FACE-NAME keys and FACE-VECTOR-DATA values. */
+ Lisp_Object face_hash_table;
/* A vector that records the entire structure of this frame's menu bar.
For the format of the data, see extensive comments in xmenu.c.
@@ -661,9 +661,9 @@ fset_condemned_scroll_bars (struct frame *f, Lisp_Object val)
f->condemned_scroll_bars = val;
}
INLINE void
-fset_face_alist (struct frame *f, Lisp_Object val)
+fset_face_hash_table (struct frame *f, Lisp_Object val)
{
- f->face_alist = val;
+ f->face_hash_table = val;
}
#if defined (HAVE_WINDOW_SYSTEM)
INLINE void
diff --git a/src/xfaces.c b/src/xfaces.c
index 7d7aff95c1..eae991d1c2 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -95,9 +95,10 @@ Copyright (C) 1993-1994, 1998-2020 Free Software Foundation, Inc.
with the symbol `face' in slot 0, and a slot for each of the face
attributes mentioned above.
- There is also a global face alist `Vface_new_frame_defaults'. Face
- definitions from this list are used to initialize faces of newly
- created frames.
+ There is also a global face map `Vface_new_frame_defaults',
+ containing conses of (FACE_ID . FACE_DEFINITION). Face definitions
+ from this table are used to initialize faces of newly created
+ frames.
A face doesn't have to specify all attributes. Those not specified
have a value of `unspecified'. Faces specifying all attributes but
@@ -1843,13 +1844,11 @@ lface_from_face_name_no_resolve (struct frame *f, Lisp_Object face_name,
Lisp_Object lface;
if (f)
- lface = assq_no_quit (face_name, f->face_alist);
+ lface = Fgethash (face_name, f->face_hash_table, Qnil);
else
- lface = assq_no_quit (face_name, Vface_new_frame_defaults);
+ lface = CDR (Fgethash (face_name, Vface_new_frame_defaults, Qnil));
- if (CONSP (lface))
- lface = XCDR (lface);
- else if (signal_p)
+ if (signal_p && NILP (lface))
signal_error ("Invalid face", face_name);
check_lface (lface);
@@ -2734,11 +2733,6 @@ DEFUN ("internal-make-lisp-face", Finternal_make_lisp_face,
/* Add a global definition if there is none. */
if (NILP (global_lface))
{
- global_lface = make_vector (LFACE_VECTOR_SIZE, Qunspecified);
- ASET (global_lface, 0, Qface);
- Vface_new_frame_defaults = Fcons (Fcons (face, global_lface),
- Vface_new_frame_defaults);
-
/* Assign the new Lisp face a unique ID. The mapping from Lisp
face id to Lisp face is given by the vector lface_id_to_name.
The mapping from Lisp face to Lisp face id is given by the
@@ -2748,9 +2742,14 @@ DEFUN ("internal-make-lisp-face", Finternal_make_lisp_face,
xpalloc (lface_id_to_name, &lface_id_to_name_size, 1, MAX_FACE_ID,
sizeof *lface_id_to_name);
+ Lisp_Object face_id = make_fixnum (next_lface_id);
lface_id_to_name[next_lface_id] = face;
- Fput (face, Qface, make_fixnum (next_lface_id));
+ Fput (face, Qface, face_id);
++next_lface_id;
+
+ global_lface = make_vector (LFACE_VECTOR_SIZE, Qunspecified);
+ ASET (global_lface, 0, Qface);
+ Fputhash (face, Fcons (face_id, global_lface), Vface_new_frame_defaults);
}
else if (f == NULL)
for (i = 1; i < LFACE_VECTOR_SIZE; ++i)
@@ -2763,7 +2762,7 @@ DEFUN ("internal-make-lisp-face", Finternal_make_lisp_face,
{
lface = make_vector (LFACE_VECTOR_SIZE, Qunspecified);
ASET (lface, 0, Qface);
- fset_face_alist (f, Fcons (Fcons (face, lface), f->face_alist));
+ Fputhash (face, lface, f->face_hash_table);
}
else
for (i = 1; i < LFACE_VECTOR_SIZE; ++i)
@@ -2924,7 +2923,7 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
f = NULL;
lface = lface_from_face_name (NULL, face, true);
- /* When updating face-new-frame-defaults, we put :ignore-defface
+ /* When updating face--new-frame-defaults, we put :ignore-defface
where the caller wants `unspecified'. This forces the frame
defaults to ignore the defface value. Otherwise, the defface
will take effect, which is generally not what is intended.
@@ -3508,7 +3507,7 @@ update_face_from_frame_parameter (struct frame *f, Lisp_Object param,
/* If there are no faces yet, give up. This is the case when called
from Fx_create_frame, and we do the necessary things later in
face-set-after-frame-defaults. */
- if (NILP (f->face_alist))
+ if (XFIXNAT (Fhash_table_count (f->face_hash_table)) == 0)
return;
if (EQ (param, Qforeground_color))
@@ -4174,14 +4173,13 @@ DEFUN ("internal-lisp-face-empty-p", Finternal_lisp_face_empty_p,
return i == LFACE_VECTOR_SIZE ? Qt : Qnil;
}
-
-DEFUN ("frame-face-alist", Fframe_face_alist, Sframe_face_alist,
+DEFUN ("frame--face-hash-table", Fframe_face_hash_table, Sframe_face_hash_table,
0, 1, 0,
- doc: /* Return an alist of frame-local faces defined on FRAME.
+ doc: /* Return a hash table of frame-local faces defined on FRAME.
For internal use only. */)
(Lisp_Object frame)
{
- return decode_live_frame (frame)->face_alist;
+ return decode_live_frame (frame)->face_hash_table;
}
@@ -6678,30 +6676,32 @@ DEFUN ("show-face-resources", Fshow_face_resources, Sshow_face_resources,
#ifdef HAVE_PDUMPER
/* All the faces defined during loadup are recorded in
- face-new-frame-defaults, with the last face first in the list. We
- need to set next_lface_id to the next face ID number, so that any
- new faces defined in this session will have face IDs different from
- those defined during loadup. We also need to set up the
- lface_id_to_name[] array for the faces that were defined during
- loadup. */
+ face-new-frame-defaults. We need to set next_lface_id to the next
+ face ID number, so that any new faces defined in this session will
+ have face IDs different from those defined during loadup. We also
+ need to set up the lface_id_to_name[] array for the faces that were
+ defined during loadup. */
void
init_xfaces (void)
{
- if (CONSP (Vface_new_frame_defaults))
+ int nfaces = XFIXNAT (Fhash_table_count (Vface_new_frame_defaults));
+ if (nfaces > 0)
{
/* Allocate the lface_id_to_name[] array. */
- lface_id_to_name_size = next_lface_id =
- XFIXNAT (Flength (Vface_new_frame_defaults));
+ lface_id_to_name_size = next_lface_id = nfaces;
lface_id_to_name = xnmalloc (next_lface_id, sizeof *lface_id_to_name);
/* Store the faces. */
- Lisp_Object tail;
- int i = next_lface_id - 1;
- for (tail = Vface_new_frame_defaults; CONSP (tail); tail = XCDR (tail))
+ struct Lisp_Hash_Table* table = XHASH_TABLE (Vface_new_frame_defaults);
+ for (ptrdiff_t idx = 0; idx < nfaces; ++idx)
{
- Lisp_Object lface = XCAR (tail);
- eassert (i >= 0);
- lface_id_to_name[i--] = XCAR (lface);
+ Lisp_Object lface = HASH_KEY (table, idx);
+ Lisp_Object face_id = CAR (HASH_VALUE (table, idx));
+ if (FIXNATP (face_id)) {
+ int id = XFIXNAT (face_id);
+ eassert (id >= 0);
+ lface_id_to_name[id] = lface;
+ }
}
}
face_attr_sym[0] = Qface;
@@ -6855,7 +6855,7 @@ syms_of_xfaces (void)
defsubr (&Sinternal_copy_lisp_face);
defsubr (&Sinternal_merge_in_global_face);
defsubr (&Sface_font);
- defsubr (&Sframe_face_alist);
+ defsubr (&Sframe_face_hash_table);
defsubr (&Sdisplay_supports_face_attributes_p);
defsubr (&Scolor_distance);
defsubr (&Sinternal_set_font_selection_order);
@@ -6879,9 +6879,11 @@ syms_of_xfaces (void)
the "specifity" of a face specification and should be let-bound
only for this purpose. */);
- DEFVAR_LISP ("face-new-frame-defaults", Vface_new_frame_defaults,
- doc: /* List of global face definitions (for internal use only.) */);
- Vface_new_frame_defaults = Qnil;
+ DEFVAR_LISP ("face--new-frame-defaults", Vface_new_frame_defaults,
+ doc: /* Hash table of global face definitions (for internal use only.) */);
+ Vface_new_frame_defaults =
+ make_hash_table (hashtest_eq, DEFAULT_HASH_SIZE, DEFAULT_REHASH_SIZE,
+ DEFAULT_REHASH_THRESHOLD, Qnil, false);
DEFVAR_LISP ("face-default-stipple", Vface_default_stipple,
doc: /* Default stipple pattern used on monochrome displays.
--
2.17.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-15 18:50 ` Clément Pit-Claudel
@ 2020-05-15 19:05 ` Eli Zaretskii
2020-05-15 19:23 ` Clément Pit-Claudel
2020-05-16 23:03 ` Juri Linkov
1 sibling, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2020-05-15 19:05 UTC (permalink / raw)
To: Clément Pit-Claudel; +Cc: 41200, juri
> Cc: 41200@debbugs.gnu.org, Juri Linkov <juri@linkov.net>
> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Fri, 15 May 2020 14:50:17 -0400
>
> >>> No, I was asking why not start with it as nil and actually make a
> >>> hash-table when we first need it.
> >>
> >> Oh, I thought it would be simpler to always have a hash table instead of having to sanity check every time.
> >
> > What is the default size of a hash-table?
>
> 65 entries, I think.
The number of basic faces is just 18. Is 65 a good initial size for
that?
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-15 14:03 ` Stefan Monnier
2020-05-15 14:34 ` Eli Zaretskii
@ 2020-05-15 19:10 ` Clément Pit-Claudel
2020-05-15 21:23 ` Stefan Monnier
1 sibling, 1 reply; 57+ messages in thread
From: Clément Pit-Claudel @ 2020-05-15 19:10 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 41200
On 15/05/2020 10.03, Stefan Monnier wrote:
>> Indeed, you're completely right; thanks! Replacing face_alist and
>> Vface_new_frame_defaults with hash tables makes the worst example
>> about 10 times faster, and with that change tooltips now take 30 to
>> 50ms to display instead of 500-600ms in my real-life use case (my
>> usual config). I have attached a patch.
>
> Oh, this is great, makes a very noticeable difference.
Thanks for testing!
> The variable's name did not say "alist", so I don't see a need to change
> it from that point of view. But I think it deserves a "--" since it's
> supposed to be internal.
Ah, that's a good point. At least, renaming it will make it clear that something changed and make it easy to support older and newer emacsen.
> A quick grep revealed:
>
> elpa/packages/context-coloring/fixtures/benchmark/faces.el: (mapcar #'car face-new-frame-defaults))
AFAICT, this is just a copy of faces.el, used to benchmark syntax highlighting (that is, this code is not run).
>> - command-execute 454 47%
> [...]
>> - face-set-after-frame-default 387 40%
>> - face-spec-recalc 374 39%
>> - make-face-x-resource-internal 296 30%
>> - set-face-attributes-from-resources 273 28%
>> - set-face-attribute-from-resource 219 22%
>
> [...]
>
>> - command-execute 768 80%
> [...]
>> - face-set-after-frame-default 674 70%
>> - face-spec-recalc 660 69%
>> - face-spec-set-2 350 36%
>> - apply 348 36%
>> - set-face-attribute 342 35%
>> - internal-set-lisp-face-attribute 342 35%
>> - frame-set-background-mode 331 34%
>> - face-spec-recalc 284 29%
>> - make-face-x-resource-internal 235 24%
>
> Both of those profiles suggest that most of the time is still spent in
> `face-spec-recalc`, so it would be worth trying harder to avoid calling
> it or to speed it up somehow (presumably with some better memozing/caching).
Interesting. I also wonder whether we could fast-track the case where the face spec is a vector full of 'undefined, since that seems to be the common case.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-15 19:05 ` Eli Zaretskii
@ 2020-05-15 19:23 ` Clément Pit-Claudel
2020-05-15 19:38 ` Eli Zaretskii
0 siblings, 1 reply; 57+ messages in thread
From: Clément Pit-Claudel @ 2020-05-15 19:23 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 41200, juri
On 15/05/2020 15.05, Eli Zaretskii wrote:
>> Cc: 41200@debbugs.gnu.org, Juri Linkov <juri@linkov.net>
>> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
>> Date: Fri, 15 May 2020 14:50:17 -0400
>>
>>>>> No, I was asking why not start with it as nil and actually make a
>>>>> hash-table when we first need it.
>>>>
>>>> Oh, I thought it would be simpler to always have a hash table instead of having to sanity check every time.
>>>
>>> What is the default size of a hash-table?
>>
>> 65 entries, I think.
>
> The number of basic faces is just 18. Is 65 a good initial size for
> that?
I think that map stores more than just the basic faces: in fact, $ emacs -Q --batch --eval '(print (length (frame-face-alist)))' prints 100, so maybe I should even make the map larger by default?
Clément.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-15 19:23 ` Clément Pit-Claudel
@ 2020-05-15 19:38 ` Eli Zaretskii
2020-05-15 19:52 ` Clément Pit-Claudel
0 siblings, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2020-05-15 19:38 UTC (permalink / raw)
To: Clément Pit-Claudel; +Cc: 41200, juri
> Cc: 41200@debbugs.gnu.org, juri@linkov.net
> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Fri, 15 May 2020 15:23:00 -0400
>
> >>> What is the default size of a hash-table?
> >>
> >> 65 entries, I think.
> >
> > The number of basic faces is just 18. Is 65 a good initial size for
> > that?
>
> I think that map stores more than just the basic faces: in fact, $ emacs -Q --batch --eval '(print (length (frame-face-alist)))' prints 100, so maybe I should even make the map larger by default?
We are miscommunicating. I was talking about the size before loadup,
i.e. in temacs when it starts up. Later, when we load preloaded
packages, the size should grow, but the hash-table takes care of that
by itself, doesn't it?
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-15 19:38 ` Eli Zaretskii
@ 2020-05-15 19:52 ` Clément Pit-Claudel
0 siblings, 0 replies; 57+ messages in thread
From: Clément Pit-Claudel @ 2020-05-15 19:52 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 41200, juri
[-- Attachment #1: Type: text/plain, Size: 1061 bytes --]
On 15/05/2020 15.38, Eli Zaretskii wrote:
>> Cc: 41200@debbugs.gnu.org, juri@linkov.net
>> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
>> Date: Fri, 15 May 2020 15:23:00 -0400
>>
>>>>> What is the default size of a hash-table?
>>>>
>>>> 65 entries, I think.
>>>
>>> The number of basic faces is just 18. Is 65 a good initial size for
>>> that?
>>
>> I think that map stores more than just the basic faces: in fact, $ emacs -Q --batch --eval '(print (length (frame-face-alist)))' prints 100, so maybe I should even make the map larger by default?
>
> We are miscommunicating. I was talking about the size before loadup,
> i.e. in temacs when it starts up.
Ah, yes. I think I mas mixing up the face_hash_table of each frame and Vface_new_frame_defaults. Attached is a patch that makes Vface_new_frame_defaults 33-entries large. Should I also change the default size for face_hash_table in struct frame?
> Later, when we load preloaded packages, the size should grow, but the
> hash-table takes care of that by itself, doesn't it?
Yes, definitely.
[-- Attachment #2: 0001-Store-frame-faces-in-hash-tables-instead-of-alists.patch --]
[-- Type: text/x-patch, Size: 16926 bytes --]
From 0d28f687046c721c4734aa57d8ae1ba5f23dd5b3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Pit-Claudel?= <clement.pitclaudel@live.com>
Date: Tue, 12 May 2020 21:48:32 -0400
Subject: [PATCH] Store frame faces in hash tables instead of alists
* src/frame.h (struct frame): Add face_hash_table, remove face_alist.
(fset_face_hash_table): New function.
(fset_face_alist): Remove.
* src/frame.c (make_frame): Initialize f->face_hash_table.
(Fmake_terminal_frame): Update to work with hash tables instead of
alists.
* src/xfaces.c (lface_from_face_name_no_resolve):
(Finternal_make_lisp_face):
(update_face_from_frame_parameter): Update to work with hash tables
instead of alists.
(Fframe_face_hash_table): New function.
(Fframe_face_alist): Move to faces.el as frame-face-alist.
(syms_of_xfaces): Add frame_face_hash_table.
* lisp/emacs-lisp/edebug.el (edebug-eval-defun):
* lisp/progmodes/elisp-mode.el (elisp--eval-defun-1):
* lisp/frame.el (frame-set-background-mode): Update to work with hash
tables instead of alists.
* lisp/faces.el (face-new-frame-defaults): Mark obsolete.
(face-list): Update to use face--new-frame-defaults.
(frame-face-alist): Moved here from src/xfaces.c.
---
lisp/custom.el | 2 +-
lisp/emacs-lisp/edebug.el | 3 +-
lisp/faces.el | 23 ++++++++--
lisp/frame.el | 2 +-
lisp/progmodes/elisp-mode.el | 3 +-
src/frame.c | 20 ++++++---
src/frame.h | 8 ++--
src/xfaces.c | 83 +++++++++++++++++++-----------------
8 files changed, 84 insertions(+), 60 deletions(-)
diff --git a/lisp/custom.el b/lisp/custom.el
index 885c486c5e..3ac8bac40f 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -898,7 +898,7 @@ custom-push-theme
;; the value to a fake theme, `changed'. If the theme is
;; later disabled, we use this to bring back the old value.
;;
- ;; For faces, we just use `face-new-frame-defaults' to
+ ;; For faces, we just use `face--new-frame-defaults' to
;; recompute when the theme is disabled.
(when (and (eq prop 'theme-value)
(boundp symbol))
diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index 78461185d3..76a75c04df 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -488,8 +488,7 @@ edebug-eval-defun
(set-default (nth 1 form) (eval (nth 2 form) lexical-binding)))
((eq (car form) 'defface)
;; Reset the face.
- (setq face-new-frame-defaults
- (assq-delete-all (nth 1 form) face-new-frame-defaults))
+ (remhash (nth 1 form) face--new-frame-defaults)
(put (nth 1 form) 'face-defface-spec nil)
(put (nth 1 form) 'face-documentation (nth 3 form))
;; See comments in `eval-defun-1' for purpose of code below
diff --git a/lisp/faces.el b/lisp/faces.el
index e707f6f4b6..bb51797a38 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -176,10 +176,27 @@ face-font-registry-alternatives
;;; Creation, copying.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+(make-obsolete-variable
+ 'face-new-frame-defaults
+ "use `face--new-frame-defaults' or `face-alist' instead."
+ "28.1")
+
+(defun frame-face-alist (&optional frame)
+ "Return an alist of frame-local faces defined on FRAME.
+This alist is a copy of the contents of `frame--face-hash-table'.
+For internal use only."
+ (declare (obsolete frame--face-hash-table "28.1"))
+ (let ((faces))
+ (maphash (lambda (face spec) (push `(,(car spec) ,face . ,(cdr spec)) faces))
+ (frame--face-hash-table frame))
+ (mapcar #'cdr (sort faces (lambda (f1 f2) (< (car f1) (car f2)))))))
(defun face-list ()
"Return a list of all defined faces."
- (mapcar #'car face-new-frame-defaults))
+ (let ((faces))
+ (maphash (lambda (face spec) (push `(,(car spec) . ,face) faces))
+ face--new-frame-defaults)
+ (mapcar #'cdr (sort faces (lambda (f1 f2) (< (car f1) (car f2)))))))
(defun make-face (face)
"Define a new face with name FACE, a symbol.
@@ -2097,7 +2114,7 @@ x-create-frame-with-faces
(defun face-set-after-frame-default (frame &optional parameters)
"Initialize the frame-local faces of FRAME.
Calculate the face definitions using the face specs, custom theme
-settings, X resources, and `face-new-frame-defaults'.
+settings, X resources, and `face--new-frame-defaults'.
Finally, apply any relevant face attributes found amongst the
frame parameters in PARAMETERS."
;; The `reverse' is so that `default' goes first.
@@ -2106,7 +2123,7 @@ face-set-after-frame-default
(progn
;; Initialize faces from face spec and custom theme.
(face-spec-recalc face frame)
- ;; Apply attributes specified by face-new-frame-defaults
+ ;; Apply attributes specified by face--new-frame-defaults
(internal-merge-in-global-face face frame))
;; Don't let invalid specs prevent frame creation.
(error nil)))
diff --git a/lisp/frame.el b/lisp/frame.el
index 6c2f774709..66b5c6aa8d 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -1227,7 +1227,7 @@ frame-set-background-mode
;; during startup with -rv on the command
;; line for the initial frame, because frames
;; are not recorded in the pdump file.
- (assq face (frame-face-alist))
+ (gethash face (frame--face-hash-table))
(face-spec-match-p face
(face-user-default-spec face)
;; FIXME: why selected-frame and
diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index b737134f90..75054b8818 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -1304,8 +1304,7 @@ elisp--eval-defun-1
((eq (car form) 'custom-declare-face)
;; Reset the face.
(let ((face-symbol (eval (nth 1 form) lexical-binding)))
- (setq face-new-frame-defaults
- (assq-delete-all face-symbol face-new-frame-defaults))
+ (remhash face--new-frame-defaults face-symbol)
(put face-symbol 'face-defface-spec nil)
(put face-symbol 'face-override-spec nil))
form)
diff --git a/src/frame.c b/src/frame.c
index c871e4fd99..0843d53559 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -946,6 +946,10 @@ make_frame (bool mini_p)
rw->total_lines = mini_p ? 9 : 10;
rw->pixel_height = rw->total_lines * FRAME_LINE_HEIGHT (f);
+ fset_face_hash_table
+ (f, make_hash_table (hashtest_eq, DEFAULT_HASH_SIZE, DEFAULT_REHASH_SIZE,
+ DEFAULT_REHASH_THRESHOLD, Qnil, false));
+
if (mini_p)
{
mw->top_line = rw->total_lines;
@@ -1254,7 +1258,7 @@ DEFUN ("make-terminal-frame", Fmake_terminal_frame, Smake_terminal_frame,
{
struct frame *f;
struct terminal *t = NULL;
- Lisp_Object frame, tem;
+ Lisp_Object frame;
struct frame *sf = SELECTED_FRAME ();
#ifdef MSDOS
@@ -1336,14 +1340,16 @@ DEFUN ("make-terminal-frame", Fmake_terminal_frame, Smake_terminal_frame,
store_in_alist (&parms, Qminibuffer, Qt);
Fmodify_frame_parameters (frame, parms);
- /* Make the frame face alist be frame-specific, so that each
+ /* Make the frame face hash be frame-specific, so that each
frame could change its face definitions independently. */
- fset_face_alist (f, Fcopy_alist (sf->face_alist));
- /* Simple Fcopy_alist isn't enough, because we need the contents of
- the vectors which are the CDRs of associations in face_alist to
+ fset_face_hash_table (f, Fcopy_hash_table (sf->face_hash_table));
+ /* Simple copy_hash_table isn't enough, because we need the contents of
+ the vectors which are the values in face_hash_table to
be copied as well. */
- for (tem = f->face_alist; CONSP (tem); tem = XCDR (tem))
- XSETCDR (XCAR (tem), Fcopy_sequence (XCDR (XCAR (tem))));
+ ptrdiff_t idx = 0;
+ struct Lisp_Hash_Table *table = XHASH_TABLE (f->face_hash_table);
+ for (idx = 0; idx < table->count; ++idx)
+ set_hash_value_slot (table, idx, Fcopy_sequence (HASH_VALUE (table, idx)));
f->can_set_window_size = true;
f->after_make_frame = true;
diff --git a/src/frame.h b/src/frame.h
index 476bac67fa..9761253085 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -158,8 +158,8 @@ #define EMACS_FRAME_H
There are four additional elements of nil at the end, to terminate. */
Lisp_Object menu_bar_items;
- /* Alist of elements (FACE-NAME . FACE-VECTOR-DATA). */
- Lisp_Object face_alist;
+ /* Hash table of FACE-NAME keys and FACE-VECTOR-DATA values. */
+ Lisp_Object face_hash_table;
/* A vector that records the entire structure of this frame's menu bar.
For the format of the data, see extensive comments in xmenu.c.
@@ -661,9 +661,9 @@ fset_condemned_scroll_bars (struct frame *f, Lisp_Object val)
f->condemned_scroll_bars = val;
}
INLINE void
-fset_face_alist (struct frame *f, Lisp_Object val)
+fset_face_hash_table (struct frame *f, Lisp_Object val)
{
- f->face_alist = val;
+ f->face_hash_table = val;
}
#if defined (HAVE_WINDOW_SYSTEM)
INLINE void
diff --git a/src/xfaces.c b/src/xfaces.c
index 7d7aff95c1..3344558a85 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -95,9 +95,10 @@ Copyright (C) 1993-1994, 1998-2020 Free Software Foundation, Inc.
with the symbol `face' in slot 0, and a slot for each of the face
attributes mentioned above.
- There is also a global face alist `Vface_new_frame_defaults'. Face
- definitions from this list are used to initialize faces of newly
- created frames.
+ There is also a global face map `Vface_new_frame_defaults',
+ containing conses of (FACE_ID . FACE_DEFINITION). Face definitions
+ from this table are used to initialize faces of newly created
+ frames.
A face doesn't have to specify all attributes. Those not specified
have a value of `unspecified'. Faces specifying all attributes but
@@ -1843,13 +1844,11 @@ lface_from_face_name_no_resolve (struct frame *f, Lisp_Object face_name,
Lisp_Object lface;
if (f)
- lface = assq_no_quit (face_name, f->face_alist);
+ lface = Fgethash (face_name, f->face_hash_table, Qnil);
else
- lface = assq_no_quit (face_name, Vface_new_frame_defaults);
+ lface = CDR (Fgethash (face_name, Vface_new_frame_defaults, Qnil));
- if (CONSP (lface))
- lface = XCDR (lface);
- else if (signal_p)
+ if (signal_p && NILP (lface))
signal_error ("Invalid face", face_name);
check_lface (lface);
@@ -2734,11 +2733,6 @@ DEFUN ("internal-make-lisp-face", Finternal_make_lisp_face,
/* Add a global definition if there is none. */
if (NILP (global_lface))
{
- global_lface = make_vector (LFACE_VECTOR_SIZE, Qunspecified);
- ASET (global_lface, 0, Qface);
- Vface_new_frame_defaults = Fcons (Fcons (face, global_lface),
- Vface_new_frame_defaults);
-
/* Assign the new Lisp face a unique ID. The mapping from Lisp
face id to Lisp face is given by the vector lface_id_to_name.
The mapping from Lisp face to Lisp face id is given by the
@@ -2748,9 +2742,14 @@ DEFUN ("internal-make-lisp-face", Finternal_make_lisp_face,
xpalloc (lface_id_to_name, &lface_id_to_name_size, 1, MAX_FACE_ID,
sizeof *lface_id_to_name);
+ Lisp_Object face_id = make_fixnum (next_lface_id);
lface_id_to_name[next_lface_id] = face;
- Fput (face, Qface, make_fixnum (next_lface_id));
+ Fput (face, Qface, face_id);
++next_lface_id;
+
+ global_lface = make_vector (LFACE_VECTOR_SIZE, Qunspecified);
+ ASET (global_lface, 0, Qface);
+ Fputhash (face, Fcons (face_id, global_lface), Vface_new_frame_defaults);
}
else if (f == NULL)
for (i = 1; i < LFACE_VECTOR_SIZE; ++i)
@@ -2763,7 +2762,7 @@ DEFUN ("internal-make-lisp-face", Finternal_make_lisp_face,
{
lface = make_vector (LFACE_VECTOR_SIZE, Qunspecified);
ASET (lface, 0, Qface);
- fset_face_alist (f, Fcons (Fcons (face, lface), f->face_alist));
+ Fputhash (face, lface, f->face_hash_table);
}
else
for (i = 1; i < LFACE_VECTOR_SIZE; ++i)
@@ -2924,7 +2923,7 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
f = NULL;
lface = lface_from_face_name (NULL, face, true);
- /* When updating face-new-frame-defaults, we put :ignore-defface
+ /* When updating face--new-frame-defaults, we put :ignore-defface
where the caller wants `unspecified'. This forces the frame
defaults to ignore the defface value. Otherwise, the defface
will take effect, which is generally not what is intended.
@@ -3508,7 +3507,7 @@ update_face_from_frame_parameter (struct frame *f, Lisp_Object param,
/* If there are no faces yet, give up. This is the case when called
from Fx_create_frame, and we do the necessary things later in
face-set-after-frame-defaults. */
- if (NILP (f->face_alist))
+ if (XFIXNAT (Fhash_table_count (f->face_hash_table)) == 0)
return;
if (EQ (param, Qforeground_color))
@@ -4174,14 +4173,13 @@ DEFUN ("internal-lisp-face-empty-p", Finternal_lisp_face_empty_p,
return i == LFACE_VECTOR_SIZE ? Qt : Qnil;
}
-
-DEFUN ("frame-face-alist", Fframe_face_alist, Sframe_face_alist,
+DEFUN ("frame--face-hash-table", Fframe_face_hash_table, Sframe_face_hash_table,
0, 1, 0,
- doc: /* Return an alist of frame-local faces defined on FRAME.
+ doc: /* Return a hash table of frame-local faces defined on FRAME.
For internal use only. */)
(Lisp_Object frame)
{
- return decode_live_frame (frame)->face_alist;
+ return decode_live_frame (frame)->face_hash_table;
}
@@ -6678,30 +6676,32 @@ DEFUN ("show-face-resources", Fshow_face_resources, Sshow_face_resources,
#ifdef HAVE_PDUMPER
/* All the faces defined during loadup are recorded in
- face-new-frame-defaults, with the last face first in the list. We
- need to set next_lface_id to the next face ID number, so that any
- new faces defined in this session will have face IDs different from
- those defined during loadup. We also need to set up the
- lface_id_to_name[] array for the faces that were defined during
- loadup. */
+ face-new-frame-defaults. We need to set next_lface_id to the next
+ face ID number, so that any new faces defined in this session will
+ have face IDs different from those defined during loadup. We also
+ need to set up the lface_id_to_name[] array for the faces that were
+ defined during loadup. */
void
init_xfaces (void)
{
- if (CONSP (Vface_new_frame_defaults))
+ int nfaces = XFIXNAT (Fhash_table_count (Vface_new_frame_defaults));
+ if (nfaces > 0)
{
/* Allocate the lface_id_to_name[] array. */
- lface_id_to_name_size = next_lface_id =
- XFIXNAT (Flength (Vface_new_frame_defaults));
+ lface_id_to_name_size = next_lface_id = nfaces;
lface_id_to_name = xnmalloc (next_lface_id, sizeof *lface_id_to_name);
/* Store the faces. */
- Lisp_Object tail;
- int i = next_lface_id - 1;
- for (tail = Vface_new_frame_defaults; CONSP (tail); tail = XCDR (tail))
+ struct Lisp_Hash_Table* table = XHASH_TABLE (Vface_new_frame_defaults);
+ for (ptrdiff_t idx = 0; idx < nfaces; ++idx)
{
- Lisp_Object lface = XCAR (tail);
- eassert (i >= 0);
- lface_id_to_name[i--] = XCAR (lface);
+ Lisp_Object lface = HASH_KEY (table, idx);
+ Lisp_Object face_id = CAR (HASH_VALUE (table, idx));
+ if (FIXNATP (face_id)) {
+ int id = XFIXNAT (face_id);
+ eassert (id >= 0);
+ lface_id_to_name[id] = lface;
+ }
}
}
face_attr_sym[0] = Qface;
@@ -6855,7 +6855,7 @@ syms_of_xfaces (void)
defsubr (&Sinternal_copy_lisp_face);
defsubr (&Sinternal_merge_in_global_face);
defsubr (&Sface_font);
- defsubr (&Sframe_face_alist);
+ defsubr (&Sframe_face_hash_table);
defsubr (&Sdisplay_supports_face_attributes_p);
defsubr (&Scolor_distance);
defsubr (&Sinternal_set_font_selection_order);
@@ -6879,9 +6879,12 @@ syms_of_xfaces (void)
the "specifity" of a face specification and should be let-bound
only for this purpose. */);
- DEFVAR_LISP ("face-new-frame-defaults", Vface_new_frame_defaults,
- doc: /* List of global face definitions (for internal use only.) */);
- Vface_new_frame_defaults = Qnil;
+ DEFVAR_LISP ("face--new-frame-defaults", Vface_new_frame_defaults,
+ doc: /* Hash table of global face definitions (for internal use only.) */);
+ Vface_new_frame_defaults =
+ /* 33 entries is enough to fit all basic faces */
+ make_hash_table (hashtest_eq, 33, DEFAULT_REHASH_SIZE,
+ DEFAULT_REHASH_THRESHOLD, Qnil, false);
DEFVAR_LISP ("face-default-stipple", Vface_default_stipple,
doc: /* Default stipple pattern used on monochrome displays.
--
2.17.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-15 19:10 ` Clément Pit-Claudel
@ 2020-05-15 21:23 ` Stefan Monnier
2020-05-16 8:45 ` martin rudalics
0 siblings, 1 reply; 57+ messages in thread
From: Stefan Monnier @ 2020-05-15 21:23 UTC (permalink / raw)
To: Clément Pit-Claudel; +Cc: 41200
> Interesting. I also wonder whether we could fast-track the case where the
> face spec is a vector full of 'undefined, since that seems to be the
> common case.
I was thinking also of the case where no face specs have changed and the
frame is "normal" (same frame properties as others, basically) so we
could re-use the faces from another frame.
[ After all, my Emacs sessions typically have dozens of frames, and only
one of them (a minibuffer-only frame) has faces setup differently
because it's configured to use a slightly larger font. ]
Stefan
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-15 21:23 ` Stefan Monnier
@ 2020-05-16 8:45 ` martin rudalics
0 siblings, 0 replies; 57+ messages in thread
From: martin rudalics @ 2020-05-16 8:45 UTC (permalink / raw)
To: Stefan Monnier, Clément Pit-Claudel; +Cc: 41200
> I was thinking also of the case where no face specs have changed and the
> frame is "normal" (same frame properties as others, basically) so we
> could re-use the faces from another frame.
That's my hope as well. For years now I'm making frames temporarily
invisible in order to avoid setting up faces for a new frame.
> [ After all, my Emacs sessions typically have dozens of frames, and only
> one of them (a minibuffer-only frame) has faces setup differently
> because it's configured to use a slightly larger font. ]
In my setup the minibuffer-only frame doesn't even do that.
martin
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-15 18:50 ` Clément Pit-Claudel
2020-05-15 19:05 ` Eli Zaretskii
@ 2020-05-16 23:03 ` Juri Linkov
2020-05-16 23:43 ` Clément Pit-Claudel
1 sibling, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2020-05-16 23:03 UTC (permalink / raw)
To: Clément Pit-Claudel; +Cc: 41200
>>>> I think the problem is that tab-line is declared a basic face, but its
>>>> defface form is not in faces.el.
>>>
>>> Ah, good catch. Current there's a defface for tab-bar in lisp/tab-bar, and since that's preloaded it works, but the defface for tab-line is in lisp/tab-line.el and so isn't preloaded.
>>> Should I move both to faces.el?
>>>
>> Yes, I think so.
>
> Thanks. I will ask Juri to confirm before moving them, because I realize now that they have a custom group.
> Juri (CC'd; hi Juri!), was there a reason to make tab-bar and tab-line basic faces? I see they are both in their own files and groups, instead of being in faces.el.
Actually, no reason other than consistency of faces belonging to the
same file where they are used. But if it will fix the technical problem,
please move them to faces.el, especially given the fact that their
counterpart tool-bar face is already defined in faces.el.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-16 23:03 ` Juri Linkov
@ 2020-05-16 23:43 ` Clément Pit-Claudel
2020-05-17 21:59 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Clément Pit-Claudel @ 2020-05-16 23:43 UTC (permalink / raw)
To: Juri Linkov; +Cc: 41200
[-- Attachment #1: Type: text/plain, Size: 1023 bytes --]
On 16/05/2020 19.03, Juri Linkov wrote:
>>>>> I think the problem is that tab-line is declared a basic face, but its
>>>>> defface form is not in faces.el.
>>>>
>>>> Ah, good catch. Current there's a defface for tab-bar in lisp/tab-bar, and since that's preloaded it works, but the defface for tab-line is in lisp/tab-line.el and so isn't preloaded.
>>>> Should I move both to faces.el?
>>>>
>>> Yes, I think so.
>>
>> Thanks. I will ask Juri to confirm before moving them, because I realize now that they have a custom group.
>> Juri (CC'd; hi Juri!), was there a reason to make tab-bar and tab-line basic faces? I see they are both in their own files and groups, instead of being in faces.el.
>
> Actually, no reason other than consistency of faces belonging to the
> same file where they are used. But if it will fix the technical problem,
> please move them to faces.el, especially given the fact that their
> counterpart tool-bar face is already defined in faces.el.
Thanks a lot. The attached patch does that.
[-- Attachment #2: 0001-Move-tab-bar-and-tab-line-to-faces.el-part-of-bug-41.patch --]
[-- Type: text/x-patch, Size: 2639 bytes --]
From 4d3349d83791a57cdc01374c82792fff6e1b8a94 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Pit-Claudel?= <clement.pitclaudel@live.com>
Date: Sat, 16 May 2020 19:36:43 -0400
Subject: [PATCH] Move tab-bar and tab-line to faces.el (part of bug#41200)
These are basic faces, so they need to be defined in
faces.el (otherwise (get 'tab-line 'face) returns 0).
* lisp/tab-bar.el (tab-bar):
* lisp/tab-line.el (tab-line): Move from here...
* lisp/faces.el (menu): ...to here.
---
lisp/faces.el | 27 +++++++++++++++++++++++++++
lisp/tab-bar.el | 13 -------------
lisp/tab-line.el | 14 --------------
3 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/lisp/faces.el b/lisp/faces.el
index bb51797a38..b495fb6e87 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -2755,6 +2755,33 @@ tool-bar
:version "21.1"
:group 'basic-faces)
+(defface tab-line
+ '((((class color) (min-colors 88))
+ :inherit variable-pitch
+ :height 0.9
+ :background "grey85"
+ :foreground "black")
+ (((class mono))
+ :background "grey")
+ (t
+ :inverse-video t))
+ "Tab line face."
+ :version "27.1"
+ :group 'basic-faces)
+
+(defface tab-bar
+ '((((class color) (min-colors 88))
+ :inherit variable-pitch
+ :background "grey85"
+ :foreground "black")
+ (((class mono))
+ :background "grey")
+ (t
+ :inverse-video t))
+ "Tab bar face."
+ :version "27.1"
+ :group 'basic-faces)
+
(defface menu
'((((type tty))
:inverse-video t)
diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index ce6d8c33dd..689481b28b 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -50,19 +50,6 @@ tab-bar-faces
:group 'faces
:version "27.1")
-(defface tab-bar
- '((((class color) (min-colors 88))
- :inherit variable-pitch
- :background "grey85"
- :foreground "black")
- (((class mono))
- :background "grey")
- (t
- :inverse-video t))
- "Tab bar face."
- :version "27.1"
- :group 'tab-bar-faces)
-
(defface tab-bar-tab
'((default
:inherit tab-bar)
diff --git a/lisp/tab-line.el b/lisp/tab-line.el
index 7a2bdc0b72..50ec40bacf 100644
--- a/lisp/tab-line.el
+++ b/lisp/tab-line.el
@@ -41,20 +41,6 @@ tab-line-faces
:group 'faces
:version "27.1")
-(defface tab-line
- '((((class color) (min-colors 88))
- :inherit variable-pitch
- :height 0.9
- :background "grey85"
- :foreground "black")
- (((class mono))
- :background "grey")
- (t
- :inverse-video t))
- "Tab line face."
- :version "27.1"
- :group 'tab-line-faces)
-
(defface tab-line-tab
'((default
:inherit tab-line)
--
2.17.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-16 23:43 ` Clément Pit-Claudel
@ 2020-05-17 21:59 ` Juri Linkov
2020-05-18 1:19 ` Clément Pit-Claudel
0 siblings, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2020-05-17 21:59 UTC (permalink / raw)
To: Clément Pit-Claudel; +Cc: 41200
> + :group 'basic-faces)
> +
> + :group 'basic-faces)
>
> - :group 'tab-bar-faces)
> -
> - :group 'tab-line-faces)
Could these faces belong to both groups?
:group 'basic-faces
:group 'tab-bar-faces
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-17 21:59 ` Juri Linkov
@ 2020-05-18 1:19 ` Clément Pit-Claudel
2020-05-19 21:48 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Clément Pit-Claudel @ 2020-05-18 1:19 UTC (permalink / raw)
To: Juri Linkov; +Cc: 41200
On 17/05/2020 17.59, Juri Linkov wrote:
>> + :group 'basic-faces)
>> +
>> + :group 'basic-faces)
>>
>> - :group 'tab-bar-faces)
>> -
>> - :group 'tab-line-faces)
>
> Could these faces belong to both groups?
>
> :group 'basic-faces
> :group 'tab-bar-faces
Yup, but that won't work well if the face is customized before the group is defined, right? Maybe the trick would be to add the group to the face when tab-bar.el and tab-bar-line.el are loaded?
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-18 1:19 ` Clément Pit-Claudel
@ 2020-05-19 21:48 ` Juri Linkov
[not found] ` <83a71z135p.fsf@gnu.org>
0 siblings, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2020-05-19 21:48 UTC (permalink / raw)
To: Clément Pit-Claudel; +Cc: 41200
>>> + :group 'basic-faces)
>>> +
>>> + :group 'basic-faces)
>>>
>>> - :group 'tab-bar-faces)
>>> -
>>> - :group 'tab-line-faces)
>>
>> Could these faces belong to both groups?
>>
>> :group 'basic-faces
>> :group 'tab-bar-faces
>
> Yup, but that won't work well if the face is customized before the
> group is defined, right? Maybe the trick would be to add the group to
> the face when tab-bar.el and tab-line.el are loaded?
Actually since tab-bar.el is pre-loaded this problem exists only for
tab-line.el.
Then adding something like
(nconc (get 'tab-line-faces 'custom-group) '((tab-line custom-face) ...))
to tab-line.el will add faces when tab-line.el are loaded.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
[not found] ` <83a71z135p.fsf@gnu.org>
@ 2020-05-23 22:47 ` Juri Linkov
2020-05-24 2:33 ` Eli Zaretskii
2020-06-08 0:21 ` Juri Linkov
1 sibling, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2020-05-23 22:47 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: cpitclaudel, 41200
>> >> Could these faces belong to both groups?
>> >>
>> >> :group 'basic-faces
>> >> :group 'tab-bar-faces
>> >
>> > Yup, but that won't work well if the face is customized before the
>> > group is defined, right? Maybe the trick would be to add the group to
>> > the face when tab-bar.el and tab-line.el are loaded?
>>
>> Actually since tab-bar.el is pre-loaded this problem exists only for
>> tab-line.el.
>>
>> Then adding something like
>>
>> (nconc (get 'tab-line-faces 'custom-group) '((tab-line custom-face) ...))
>>
>> to tab-line.el will add faces when tab-line.el are loaded.
>
> Could you guys please finalize these issues? I'd like to install
> these changes on master, for which I'd need a patch that covers both
> the changes in the face storage and the fix for the tab-line face.
>
> Actually, the problem with the tab-line face will probably need to be
> fixed on emacs-27.
I don't recommend using the trick with nconc in emacs-27. It would be
less risky if Clément will just move tab faces to faces.el in emacs-27.
Or may I suggest to preload tab-line.el in emacs-27?
That will solve the problem as well.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-23 22:47 ` Juri Linkov
@ 2020-05-24 2:33 ` Eli Zaretskii
2020-05-24 21:50 ` Juri Linkov
0 siblings, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2020-05-24 2:33 UTC (permalink / raw)
To: Juri Linkov; +Cc: cpitclaudel, 41200
> From: Juri Linkov <juri@linkov.net>
> Cc: cpitclaudel@gmail.com, 41200@debbugs.gnu.org
> Date: Sun, 24 May 2020 01:47:28 +0300
>
> > Actually, the problem with the tab-line face will probably need to be
> > fixed on emacs-27.
>
> I don't recommend using the trick with nconc in emacs-27. It would be
> less risky if Clément will just move tab faces to faces.el in emacs-27.
Agreed.
> Or may I suggest to preload tab-line.el in emacs-27?
Doesn't sound justified to me. I don't expect this feature to be so
popular as to always have it available.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-24 2:33 ` Eli Zaretskii
@ 2020-05-24 21:50 ` Juri Linkov
0 siblings, 0 replies; 57+ messages in thread
From: Juri Linkov @ 2020-05-24 21:50 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: cpitclaudel, 41200
>> Or may I suggest to preload tab-line.el in emacs-27?
>
> Doesn't sound justified to me. I don't expect this feature to be so
> popular as to always have it available.
Agreed, better to move deffaces to faces.el.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
[not found] ` <83a71z135p.fsf@gnu.org>
2020-05-23 22:47 ` Juri Linkov
@ 2020-06-08 0:21 ` Juri Linkov
2020-06-20 7:47 ` Eli Zaretskii
1 sibling, 1 reply; 57+ messages in thread
From: Juri Linkov @ 2020-06-08 0:21 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: cpitclaudel, 41200
>> >>> + :group 'basic-faces)
>> >>> + :group 'basic-faces)
>> >>>
>> >>> - :group 'tab-bar-faces)
>> >>> - :group 'tab-line-faces)
>> >>
>> >> Could these faces belong to both groups?
>> >>
>> >> :group 'basic-faces
>> >> :group 'tab-bar-faces
>> >
>> > Yup, but that won't work well if the face is customized before the
>> > group is defined, right? Maybe the trick would be to add the group to
>> > the face when tab-bar.el and tab-line.el are loaded?
>>
>> Actually since tab-bar.el is pre-loaded this problem exists only for
>> tab-line.el.
>>
>> Then adding something like
>>
>> (nconc (get 'tab-line-faces 'custom-group) '((tab-line custom-face) ...))
>>
>> to tab-line.el will add faces when tab-line.el are loaded.
>
> Could you guys please finalize these issues? I'd like to install
> these changes on master, for which I'd need a patch that covers both
> the changes in the face storage and the fix for the tab-line face.
>
> Actually, the problem with the tab-line face will probably need to be
> fixed on emacs-27.
I never used the second argument MEMBERS of 'defgroup', but it's exactly
what is needed. Now the problem with the tab-line face is fixed on emacs-27
in commit 6eb18a950d.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-06-08 0:21 ` Juri Linkov
@ 2020-06-20 7:47 ` Eli Zaretskii
2020-06-20 16:55 ` Clément Pit-Claudel
0 siblings, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2020-06-20 7:47 UTC (permalink / raw)
To: Juri Linkov, Clément Pit-Claudel; +Cc: 41200
> From: Juri Linkov <juri@linkov.net>
> Cc: cpitclaudel@gmail.com, 41200@debbugs.gnu.org
> Date: Mon, 08 Jun 2020 03:21:58 +0300
>
> > Could you guys please finalize these issues? I'd like to install
> > these changes on master, for which I'd need a patch that covers both
> > the changes in the face storage and the fix for the tab-line face.
> >
> > Actually, the problem with the tab-line face will probably need to be
> > fixed on emacs-27.
>
> I never used the second argument MEMBERS of 'defgroup', but it's exactly
> what is needed. Now the problem with the tab-line face is fixed on emacs-27
> in commit 6eb18a950d.
Thanks.
Clément, would you like to rebase your patch on the current master and
resubmit? I think we are ready for installing it.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-06-20 7:47 ` Eli Zaretskii
@ 2020-06-20 16:55 ` Clément Pit-Claudel
2020-07-04 7:58 ` Eli Zaretskii
0 siblings, 1 reply; 57+ messages in thread
From: Clément Pit-Claudel @ 2020-06-20 16:55 UTC (permalink / raw)
To: Eli Zaretskii, Juri Linkov; +Cc: 41200
On 20/06/2020 03.47, Eli Zaretskii wrote:
>> From: Juri Linkov <juri@linkov.net>
>> Cc: cpitclaudel@gmail.com, 41200@debbugs.gnu.org
>> Date: Mon, 08 Jun 2020 03:21:58 +0300
>>
>>> Could you guys please finalize these issues? I'd like to install
>>> these changes on master, for which I'd need a patch that covers both
>>> the changes in the face storage and the fix for the tab-line face.
>>>
>>> Actually, the problem with the tab-line face will probably need to be
>>> fixed on emacs-27.
>>
>> I never used the second argument MEMBERS of 'defgroup', but it's exactly
>> what is needed. Now the problem with the tab-line face is fixed on emacs-27
>> in commit 6eb18a950d.
>
> Thanks.
>
> Clément, would you like to rebase your patch on the current master and
> resubmit? I think we are ready for installing it.
Yes, sorry for the delay. I will try to do this today. Thanks for your patience.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-06-20 16:55 ` Clément Pit-Claudel
@ 2020-07-04 7:58 ` Eli Zaretskii
2020-09-13 2:53 ` Benson Chu
0 siblings, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2020-07-04 7:58 UTC (permalink / raw)
To: Clément Pit-Claudel; +Cc: 41200, juri
> Cc: 41200@debbugs.gnu.org
> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Sat, 20 Jun 2020 12:55:18 -0400
>
> > Clément, would you like to rebase your patch on the current master and
> > resubmit? I think we are ready for installing it.
>
> Yes, sorry for the delay. I will try to do this today. Thanks for your patience.
Ping!
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-07-04 7:58 ` Eli Zaretskii
@ 2020-09-13 2:53 ` Benson Chu
0 siblings, 0 replies; 57+ messages in thread
From: Benson Chu @ 2020-09-13 2:53 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 41200
Hey all,
I've been using this patch for 3 months now, and it makes my Emacs much
snappier! However, every time I want to update master, I have to rebase
this patch up. Is there anything preventing this changeset from landing
on master?
Is there anything I can do to help?
Also, I think there's a bug in the latest patch on line 1308 of
elisp-mode.el. The arguments for the call to remhash are reversed.
Thanks!
Benson
Eli Zaretskii <eliz@gnu.org> writes:
>> Cc: 41200@debbugs.gnu.org
>> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
>> Date: Sat, 20 Jun 2020 12:55:18 -0400
>>
>> > Clément, would you like to rebase your patch on the current master and
>> > resubmit? I think we are ready for installing it.
>>
>> Yes, sorry for the delay. I will try to do this today. Thanks for your patience.
>
> Ping!
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2020-05-12 4:30 bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined Clément Pit-Claudel
` (2 preceding siblings ...)
2020-05-15 14:03 ` Stefan Monnier
@ 2021-04-06 6:35 ` Jashank Jeremy
2021-04-06 12:30 ` Eli Zaretskii
2021-04-23 3:56 ` Stefan Monnier
3 siblings, 2 replies; 57+ messages in thread
From: Jashank Jeremy @ 2021-04-06 6:35 UTC (permalink / raw)
To: Emacs bug41200; +Cc: Clément Pit-Claudel
[-- Attachment #1.1: Type: text/plain, Size: 520 bytes --]
Hello,
I have updated this patch to make it compile and behave on Emacs master
at 1d93540371aadec8f877bd781267d38d411c40a0 (current a few hours ago).
The updated patch is attached.
I queried Clément, who has ok'd my shepherding this patch along.
CPU profile data I've gathered suggests this change has almost entirely
stamped out the face-related hot-spots I'd seen previously. Hooray!
Cheers,
~jashank
--
Jashank Jeremy
WWW jashankj.space
pgp FE4D6AA8 3736591C FAD30D97 DCE96CF5 8D931E04
[-- Attachment #1.2: 0001-Store-frame-faces-in-hash-tables-instead-of-alists-V9.patch --]
[-- Type: text/x-patch, Size: 19048 bytes --]
From 79586b3ea1100a0f8ca50be8b4dd62684558408e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Pit-Claudel?= <clement.pitclaudel@live.com>
Date: Tue, 12 May 2020 21:48:32 -0400
Subject: [PATCH] Store frame faces in hash tables instead of alists
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* src/frame.h (struct frame): Add face_hash_table, remove face_alist.
(fset_face_hash_table): New function.
(fset_face_alist): Remove.
* src/frame.c (make_frame): Initialize f->face_hash_table.
(Fmake_terminal_frame): Update to work with hash tables instead of
alists.
* src/xfaces.c (lface_from_face_name_no_resolve):
(Finternal_make_lisp_face):
(update_face_from_frame_parameter): Update to work with hash tables
instead of alists.
(Fframe_face_hash_table): New function.
(Fframe_face_alist): Move to faces.el as frame-face-alist.
(syms_of_xfaces): Add frame_face_hash_table.
* lisp/progmodes/elisp-mode.el (elisp--eval-defun-1):
* lisp/frame.el (frame-set-background-mode): Update to work with hash
tables instead of alists.
* lisp/faces.el (face-new-frame-defaults): Mark obsolete.
(face-list): Update to use face--new-frame-defaults.
(frame-face-alist): Moved here from src/xfaces.c.
(x-create-frame-with-faces): Update to handle subtle semantic change
to how frame faces propagate, which otherwise breaks frame creation
with reverse video enabled.
;; Patch history:
;;
;; V1 (2020-05-12 22:41:24 -0400), (Bug#41200) message 29.
;; Authored-by: Clément Pit-Claudel <clement.pitclaudel <at> live.com>
;;
;; V2 (2020-05-13 11:13:27 -0400), (Bug#41200) message 38.
;; Fixes compilation error.
;; Reported-by: martin rudalics <rudalics <at> gmx.at>
;; Authored-by: Clément Pit-Claudel <clement.pitclaudel <at> live.com>
;;
;; V3 (2020-05-15 10:59:36 -0400), (Bug#41200) message 55.
;; Amendments to resolve issues raised or resolved by EliZ.
;; * Code-style cleanup.
;; * Rename `face_hash' to `face_map'.
;; Suggested-by: Eli Zaretskii <eliz <at> gnu.org>
;; Authored-by: Clément Pit-Claudel <clement.pitclaudel <at> live.com>
;;
;; V4 (2020-05-15 14:50:17 -0400), (Bug#41200) message 70.
;; * Settle on name `face_hash_table'.
;; * Rename `face-new-frame-defaults' to `face--new-frame-defaults'.
;; * Allow multiple variants of a face to exist.
;; Suggested-by: Eli Zaretskii <eliz <at> gnu.org>
;; Authored-by: Clément Pit-Claudel <clement.pitclaudel <at> live.com>
;;
;; V5 (2020-05-15 14:50:17 -0400), (Bug#41200) message 85.
;; Lower the size of `face--new-frame-defaults'.
;; Suggested-by: Eli Zaretskii <eliz <at> gnu.org>
;; Authored-by: Clément Pit-Claudel <clement.pitclaudel <at> live.com>
;;
;; V6 (2021-04-05 17:10:55 +1000).
;; Rebase onto master; move hunks around.
;; * Changes to `edebug.el' dropped.
;; Authored-by: Jashank Jeremy <jashank <at> rulingia.com.au>
;;
;; V7 (2021-04-06 12:31:55 +1000).
;; Fix `frame-face-alist' to cope with changes from V4.
;; Authored-by: Jashank Jeremy <jashank <at> rulingia.com.au>
;;
;; V8 (2021-04-06 13:13:33 +1000).
;; There is a subtle change to face propagation semantics at face
;; creation time, which stops face frame creation working at all
;; only if reverse video is enabled.
;; Authored-by: Jashank Jeremy <jashank <at> rulingia.com.au>
;;
;; V9 (2021-04-06 16:17:53 +1000).
;; Correct argument order of `remhash' in `elisp-mode.el'.
;; Reported-by: Benson Chu <bensonchu457 <at> gmail.com>
;; Authored-by: Jashank Jeremy <jashank <at> rulingia.com.au>
---
lisp/custom.el | 2 +-
lisp/faces.el | 27 ++++++++++--
lisp/frame.el | 2 +-
lisp/progmodes/elisp-mode.el | 3 +-
src/frame.c | 20 ++++++---
src/frame.h | 8 ++--
src/xfaces.c | 83 +++++++++++++++++++-----------------
7 files changed, 86 insertions(+), 59 deletions(-)
diff --git a/lisp/custom.el b/lisp/custom.el
index 85e5d65ffb..5b071ef311 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -906,7 +906,7 @@ custom-push-theme
;; the value to a fake theme, `changed'. If the theme is
;; later disabled, we use this to bring back the old value.
;;
- ;; For faces, we just use `face-new-frame-defaults' to
+ ;; For faces, we just use `face--new-frame-defaults' to
;; recompute when the theme is disabled.
(when (and (eq prop 'theme-value)
(boundp symbol))
diff --git a/lisp/faces.el b/lisp/faces.el
index 42f4cddfb1..2127b90c86 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -176,10 +176,29 @@ face-font-registry-alternatives
;;; Creation, copying.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+(make-obsolete-variable
+ 'face-new-frame-defaults
+ "use `face--new-frame-defaults' or `face-alist' instead."
+ "28.1")
+
+(defun frame-face-alist (&optional frame)
+ "Return an alist of frame-local faces defined on FRAME.
+This alist is a copy of the contents of `frame--face-hash-table'.
+For internal use only."
+ (declare (obsolete frame--face-hash-table "28.1"))
+ (let ((faces))
+ (maphash (lambda (face spec)
+ (let ((face-id (car (gethash face face--new-frame-defaults))))
+ (push `(,face-id ,face . ,spec) faces)))
+ (frame--face-hash-table frame))
+ (mapcar #'cdr (sort faces (lambda (f1 f2) (< (car f1) (car f2)))))))
(defun face-list ()
"Return a list of all defined faces."
- (mapcar #'car face-new-frame-defaults))
+ (let ((faces))
+ (maphash (lambda (face spec) (push `(,(car spec) . ,face) faces))
+ face--new-frame-defaults)
+ (mapcar #'cdr (sort faces (lambda (f1 f2) (< (car f1) (car f2)))))))
(defun make-face (face)
"Define a new face with name FACE, a symbol.
@@ -2115,9 +2134,9 @@ x-create-frame-with-faces
(unwind-protect
(progn
(x-setup-function-keys frame)
+ (face-set-after-frame-default frame parameters)
(x-handle-reverse-video frame parameters)
(frame-set-background-mode frame t)
- (face-set-after-frame-default frame parameters)
(if (null visibility-spec)
(make-frame-visible frame)
(modify-frame-parameters frame (list visibility-spec)))
@@ -2129,7 +2148,7 @@ x-create-frame-with-faces
(defun face-set-after-frame-default (frame &optional parameters)
"Initialize the frame-local faces of FRAME.
Calculate the face definitions using the face specs, custom theme
-settings, X resources, and `face-new-frame-defaults'.
+settings, X resources, and `face--new-frame-defaults'.
Finally, apply any relevant face attributes found amongst the
frame parameters in PARAMETERS."
;; The `reverse' is so that `default' goes first.
@@ -2138,7 +2157,7 @@ face-set-after-frame-default
(progn
;; Initialize faces from face spec and custom theme.
(face-spec-recalc face frame)
- ;; Apply attributes specified by face-new-frame-defaults
+ ;; Apply attributes specified by face--new-frame-defaults
(internal-merge-in-global-face face frame))
;; Don't let invalid specs prevent frame creation.
(error nil)))
diff --git a/lisp/frame.el b/lisp/frame.el
index 2b6e4a60b8..0d78e2fb39 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -1247,7 +1247,7 @@ frame-set-background-mode
;; during startup with -rv on the command
;; line for the initial frame, because frames
;; are not recorded in the pdump file.
- (assq face (frame-face-alist frame))
+ (gethash face (frame--face-hash-table))
(face-spec-match-p face
(face-user-default-spec face)
frame)))
diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index 8ade718640..aa5801a7ca 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -1309,8 +1309,7 @@ elisp--eval-defun-1
((eq (car form) 'custom-declare-face)
;; Reset the face.
(let ((face-symbol (eval (nth 1 form) lexical-binding)))
- (setq face-new-frame-defaults
- (assq-delete-all face-symbol face-new-frame-defaults))
+ (remhash face-symbol face--new-frame-defaults)
(put face-symbol 'face-defface-spec nil)
(put face-symbol 'face-override-spec nil))
form)
diff --git a/src/frame.c b/src/frame.c
index bbdc3b5599..c8f341b829 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -946,6 +946,10 @@ make_frame (bool mini_p)
rw->total_lines = FRAME_LINES (f) - (mini_p ? 1 : 0);
rw->pixel_height = rw->total_lines * FRAME_LINE_HEIGHT (f);
+ fset_face_hash_table
+ (f, make_hash_table (hashtest_eq, DEFAULT_HASH_SIZE, DEFAULT_REHASH_SIZE,
+ DEFAULT_REHASH_THRESHOLD, Qnil, false));
+
if (mini_p)
{
mw->top_line = rw->total_lines;
@@ -1254,7 +1258,7 @@ DEFUN ("make-terminal-frame", Fmake_terminal_frame, Smake_terminal_frame,
{
struct frame *f;
struct terminal *t = NULL;
- Lisp_Object frame, tem;
+ Lisp_Object frame;
struct frame *sf = SELECTED_FRAME ();
#ifdef MSDOS
@@ -1336,14 +1340,16 @@ DEFUN ("make-terminal-frame", Fmake_terminal_frame, Smake_terminal_frame,
store_in_alist (&parms, Qminibuffer, Qt);
Fmodify_frame_parameters (frame, parms);
- /* Make the frame face alist be frame-specific, so that each
+ /* Make the frame face hash be frame-specific, so that each
frame could change its face definitions independently. */
- fset_face_alist (f, Fcopy_alist (sf->face_alist));
- /* Simple Fcopy_alist isn't enough, because we need the contents of
- the vectors which are the CDRs of associations in face_alist to
+ fset_face_hash_table (f, Fcopy_hash_table (sf->face_hash_table));
+ /* Simple copy_hash_table isn't enough, because we need the contents of
+ the vectors which are the values in face_hash_table to
be copied as well. */
- for (tem = f->face_alist; CONSP (tem); tem = XCDR (tem))
- XSETCDR (XCAR (tem), Fcopy_sequence (XCDR (XCAR (tem))));
+ ptrdiff_t idx = 0;
+ struct Lisp_Hash_Table *table = XHASH_TABLE (f->face_hash_table);
+ for (idx = 0; idx < table->count; ++idx)
+ set_hash_value_slot (table, idx, Fcopy_sequence (HASH_VALUE (table, idx)));
f->can_set_window_size = true;
f->after_make_frame = true;
diff --git a/src/frame.h b/src/frame.h
index 0fd95e4dd3..1c297921f7 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -158,8 +158,8 @@ #define EMACS_FRAME_H
There are four additional elements of nil at the end, to terminate. */
Lisp_Object menu_bar_items;
- /* Alist of elements (FACE-NAME . FACE-VECTOR-DATA). */
- Lisp_Object face_alist;
+ /* Hash table of FACE-NAME keys and FACE-VECTOR-DATA values. */
+ Lisp_Object face_hash_table;
/* A vector that records the entire structure of this frame's menu bar.
For the format of the data, see extensive comments in xmenu.c.
@@ -665,9 +665,9 @@ fset_condemned_scroll_bars (struct frame *f, Lisp_Object val)
f->condemned_scroll_bars = val;
}
INLINE void
-fset_face_alist (struct frame *f, Lisp_Object val)
+fset_face_hash_table (struct frame *f, Lisp_Object val)
{
- f->face_alist = val;
+ f->face_hash_table = val;
}
#if defined (HAVE_WINDOW_SYSTEM)
INLINE void
diff --git a/src/xfaces.c b/src/xfaces.c
index ab4440f46a..d86adef62c 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -95,9 +95,10 @@ Copyright (C) 1993-1994, 1998-2021 Free Software Foundation, Inc.
with the symbol `face' in slot 0, and a slot for each of the face
attributes mentioned above.
- There is also a global face alist `Vface_new_frame_defaults'. Face
- definitions from this list are used to initialize faces of newly
- created frames.
+ There is also a global face map `Vface_new_frame_defaults',
+ containing conses of (FACE_ID . FACE_DEFINITION). Face definitions
+ from this table are used to initialize faces of newly created
+ frames.
A face doesn't have to specify all attributes. Those not specified
have a value of `unspecified'. Faces specifying all attributes but
@@ -1962,13 +1963,11 @@ lface_from_face_name_no_resolve (struct frame *f, Lisp_Object face_name,
Lisp_Object lface;
if (f)
- lface = assq_no_quit (face_name, f->face_alist);
+ lface = Fgethash (face_name, f->face_hash_table, Qnil);
else
- lface = assq_no_quit (face_name, Vface_new_frame_defaults);
+ lface = CDR (Fgethash (face_name, Vface_new_frame_defaults, Qnil));
- if (CONSP (lface))
- lface = XCDR (lface);
- else if (signal_p)
+ if (signal_p && NILP (lface))
signal_error ("Invalid face", face_name);
check_lface (lface);
@@ -2867,11 +2866,6 @@ DEFUN ("internal-make-lisp-face", Finternal_make_lisp_face,
/* Add a global definition if there is none. */
if (NILP (global_lface))
{
- global_lface = make_vector (LFACE_VECTOR_SIZE, Qunspecified);
- ASET (global_lface, 0, Qface);
- Vface_new_frame_defaults = Fcons (Fcons (face, global_lface),
- Vface_new_frame_defaults);
-
/* Assign the new Lisp face a unique ID. The mapping from Lisp
face id to Lisp face is given by the vector lface_id_to_name.
The mapping from Lisp face to Lisp face id is given by the
@@ -2881,9 +2875,14 @@ DEFUN ("internal-make-lisp-face", Finternal_make_lisp_face,
xpalloc (lface_id_to_name, &lface_id_to_name_size, 1, MAX_FACE_ID,
sizeof *lface_id_to_name);
+ Lisp_Object face_id = make_fixnum (next_lface_id);
lface_id_to_name[next_lface_id] = face;
- Fput (face, Qface, make_fixnum (next_lface_id));
+ Fput (face, Qface, face_id);
++next_lface_id;
+
+ global_lface = make_vector (LFACE_VECTOR_SIZE, Qunspecified);
+ ASET (global_lface, 0, Qface);
+ Fputhash (face, Fcons (face_id, global_lface), Vface_new_frame_defaults);
}
else if (f == NULL)
for (i = 1; i < LFACE_VECTOR_SIZE; ++i)
@@ -2896,7 +2895,7 @@ DEFUN ("internal-make-lisp-face", Finternal_make_lisp_face,
{
lface = make_vector (LFACE_VECTOR_SIZE, Qunspecified);
ASET (lface, 0, Qface);
- fset_face_alist (f, Fcons (Fcons (face, lface), f->face_alist));
+ Fputhash (face, lface, f->face_hash_table);
}
else
for (i = 1; i < LFACE_VECTOR_SIZE; ++i)
@@ -3057,7 +3056,7 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
f = NULL;
lface = lface_from_face_name (NULL, face, true);
- /* When updating face-new-frame-defaults, we put :ignore-defface
+ /* When updating face--new-frame-defaults, we put :ignore-defface
where the caller wants `unspecified'. This forces the frame
defaults to ignore the defface value. Otherwise, the defface
will take effect, which is generally not what is intended.
@@ -3642,7 +3641,7 @@ update_face_from_frame_parameter (struct frame *f, Lisp_Object param,
/* If there are no faces yet, give up. This is the case when called
from Fx_create_frame, and we do the necessary things later in
face-set-after-frame-defaults. */
- if (NILP (f->face_alist))
+ if (XFIXNAT (Fhash_table_count (f->face_hash_table)) == 0)
return;
if (EQ (param, Qforeground_color))
@@ -4308,14 +4307,13 @@ DEFUN ("internal-lisp-face-empty-p", Finternal_lisp_face_empty_p,
return i == LFACE_VECTOR_SIZE ? Qt : Qnil;
}
-
-DEFUN ("frame-face-alist", Fframe_face_alist, Sframe_face_alist,
+DEFUN ("frame--face-hash-table", Fframe_face_hash_table, Sframe_face_hash_table,
0, 1, 0,
- doc: /* Return an alist of frame-local faces defined on FRAME.
+ doc: /* Return a hash table of frame-local faces defined on FRAME.
For internal use only. */)
(Lisp_Object frame)
{
- return decode_live_frame (frame)->face_alist;
+ return decode_live_frame (frame)->face_hash_table;
}
@@ -6832,30 +6830,32 @@ DEFUN ("show-face-resources", Fshow_face_resources, Sshow_face_resources,
#ifdef HAVE_PDUMPER
/* All the faces defined during loadup are recorded in
- face-new-frame-defaults, with the last face first in the list. We
- need to set next_lface_id to the next face ID number, so that any
- new faces defined in this session will have face IDs different from
- those defined during loadup. We also need to set up the
- lface_id_to_name[] array for the faces that were defined during
- loadup. */
+ face-new-frame-defaults. We need to set next_lface_id to the next
+ face ID number, so that any new faces defined in this session will
+ have face IDs different from those defined during loadup. We also
+ need to set up the lface_id_to_name[] array for the faces that were
+ defined during loadup. */
void
init_xfaces (void)
{
- if (CONSP (Vface_new_frame_defaults))
+ int nfaces = XFIXNAT (Fhash_table_count (Vface_new_frame_defaults));
+ if (nfaces > 0)
{
/* Allocate the lface_id_to_name[] array. */
- lface_id_to_name_size = next_lface_id =
- XFIXNAT (Flength (Vface_new_frame_defaults));
+ lface_id_to_name_size = next_lface_id = nfaces;
lface_id_to_name = xnmalloc (next_lface_id, sizeof *lface_id_to_name);
/* Store the faces. */
- Lisp_Object tail;
- int i = next_lface_id - 1;
- for (tail = Vface_new_frame_defaults; CONSP (tail); tail = XCDR (tail))
+ struct Lisp_Hash_Table* table = XHASH_TABLE (Vface_new_frame_defaults);
+ for (ptrdiff_t idx = 0; idx < nfaces; ++idx)
{
- Lisp_Object lface = XCAR (tail);
- eassert (i >= 0);
- lface_id_to_name[i--] = XCAR (lface);
+ Lisp_Object lface = HASH_KEY (table, idx);
+ Lisp_Object face_id = CAR (HASH_VALUE (table, idx));
+ if (FIXNATP (face_id)) {
+ int id = XFIXNAT (face_id);
+ eassert (id >= 0);
+ lface_id_to_name[id] = lface;
+ }
}
}
face_attr_sym[0] = Qface;
@@ -7011,7 +7011,7 @@ syms_of_xfaces (void)
defsubr (&Sinternal_copy_lisp_face);
defsubr (&Sinternal_merge_in_global_face);
defsubr (&Sface_font);
- defsubr (&Sframe_face_alist);
+ defsubr (&Sframe_face_hash_table);
defsubr (&Sdisplay_supports_face_attributes_p);
defsubr (&Scolor_distance);
defsubr (&Sinternal_set_font_selection_order);
@@ -7035,9 +7035,12 @@ syms_of_xfaces (void)
the "specifity" of a face specification and should be let-bound
only for this purpose. */);
- DEFVAR_LISP ("face-new-frame-defaults", Vface_new_frame_defaults,
- doc: /* List of global face definitions (for internal use only.) */);
- Vface_new_frame_defaults = Qnil;
+ DEFVAR_LISP ("face--new-frame-defaults", Vface_new_frame_defaults,
+ doc: /* Hash table of global face definitions (for internal use only.) */);
+ Vface_new_frame_defaults =
+ /* 33 entries is enough to fit all basic faces */
+ make_hash_table (hashtest_eq, 33, DEFAULT_REHASH_SIZE,
+ DEFAULT_REHASH_THRESHOLD, Qnil, false);
DEFVAR_LISP ("face-default-stipple", Vface_default_stipple,
doc: /* Default stipple pattern used on monochrome displays.
--
2.31.1
[-- Attachment #2: OpenPGP Digital Signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2021-04-06 6:35 ` Jashank Jeremy
@ 2021-04-06 12:30 ` Eli Zaretskii
2021-04-06 15:07 ` Clément Pit-Claudel
2021-04-23 3:56 ` Stefan Monnier
1 sibling, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2021-04-06 12:30 UTC (permalink / raw)
To: Jashank Jeremy; +Cc: 41200, cpitclaudel
> Date: Tue, 06 Apr 2021 16:35:28 +1000
> From: Jashank Jeremy <jashank@rulingia.com.au>
> Cc: Clément Pit-Claudel <cpitclaudel@gmail.com>
>
> I have updated this patch to make it compile and behave on Emacs master
> at 1d93540371aadec8f877bd781267d38d411c40a0 (current a few hours ago).
> The updated patch is attached.
>
> I queried Clément, who has ok'd my shepherding this patch along.
>
> CPU profile data I've gathered suggests this change has almost entirely
> stamped out the face-related hot-spots I'd seen previously. Hooray!
Yeah, it's a pity Clément dropped the ball on this one.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2021-04-06 12:30 ` Eli Zaretskii
@ 2021-04-06 15:07 ` Clément Pit-Claudel
2021-04-06 15:50 ` Eli Zaretskii
0 siblings, 1 reply; 57+ messages in thread
From: Clément Pit-Claudel @ 2021-04-06 15:07 UTC (permalink / raw)
To: Eli Zaretskii, Jashank Jeremy; +Cc: 41200
On 4/6/21 8:30 AM, Eli Zaretskii wrote:
>> Date: Tue, 06 Apr 2021 16:35:28 +1000
>> From: Jashank Jeremy <jashank@rulingia.com.au>
>> Cc: Clément Pit-Claudel <cpitclaudel@gmail.com>
>>
>> I have updated this patch to make it compile and behave on Emacs master
>> at 1d93540371aadec8f877bd781267d38d411c40a0 (current a few hours ago).
>> The updated patch is attached.
>>
>> I queried Clément, who has ok'd my shepherding this patch along.
>>
>> CPU profile data I've gathered suggests this change has almost entirely
>> stamped out the face-related hot-spots I'd seen previously. Hooray!
>
> Yeah, it's a pity Clément dropped the ball on this one.
Sorry Eli, life got in the way. I'm glad that Jashank is taking care of it :)
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2021-04-06 15:07 ` Clément Pit-Claudel
@ 2021-04-06 15:50 ` Eli Zaretskii
0 siblings, 0 replies; 57+ messages in thread
From: Eli Zaretskii @ 2021-04-06 15:50 UTC (permalink / raw)
To: Clément Pit-Claudel; +Cc: jashank, 41200
> Cc: 41200@debbugs.gnu.org
> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Tue, 6 Apr 2021 11:07:00 -0400
>
> Sorry Eli, life got in the way. I'm glad that Jashank is taking care of it :)
No need to apologize, we all have that problem sometimes.
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2021-04-06 6:35 ` Jashank Jeremy
2021-04-06 12:30 ` Eli Zaretskii
@ 2021-04-23 3:56 ` Stefan Monnier
2021-05-12 20:29 ` Lars Ingebrigtsen
1 sibling, 1 reply; 57+ messages in thread
From: Stefan Monnier @ 2021-04-23 3:56 UTC (permalink / raw)
To: Jashank Jeremy; +Cc: Emacs bug41200, Clément Pit-Claudel
> I have updated this patch to make it compile and behave on Emacs master
> at 1d93540371aadec8f877bd781267d38d411c40a0 (current a few hours ago).
> The updated patch is attached.
I just tried it on Emacs's `master` but for me it results in
src/emacs -Q
opening up with most faces that don't use colors: the *scratch* buffer's
comments are displayed in black+bold+italic, the mode-line has
a black background. A call of the form `(error "foo")` has foo display
in black+italic and the `error symbol is displayed in reverse video
(black background).
OTOH, the `M-x` that is displayed when I hit `M-x` seems to have the
usual blueish color.
Stefan
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2021-04-23 3:56 ` Stefan Monnier
@ 2021-05-12 20:29 ` Lars Ingebrigtsen
2021-05-13 3:56 ` Jashank Jeremy
0 siblings, 1 reply; 57+ messages in thread
From: Lars Ingebrigtsen @ 2021-05-12 20:29 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Jashank Jeremy, Emacs bug41200, Clément Pit-Claudel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
> I just tried it on Emacs's `master` but for me it results in
>
> src/emacs -Q
>
> opening up with most faces that don't use colors: the *scratch* buffer's
> comments are displayed in black+bold+italic, the mode-line has
> a black background. A call of the form `(error "foo")` has foo display
> in black+italic and the `error symbol is displayed in reverse video
> (black background).
I tried the patch, too, and I'm seeing the same -- most faces are
different than they used to be, which is surely not what's supposed to
happen.
Jashank, have to continued working on this patch?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2021-05-12 20:29 ` Lars Ingebrigtsen
@ 2021-05-13 3:56 ` Jashank Jeremy
2021-05-13 9:15 ` Lars Ingebrigtsen
2021-07-21 14:02 ` Lars Ingebrigtsen
0 siblings, 2 replies; 57+ messages in thread
From: Jashank Jeremy @ 2021-05-13 3:56 UTC (permalink / raw)
To: Lars Ingebrigtsen, Stefan Monnier
Cc: Emacs bug41200, Clément Pit-Claudel
[-- Attachment #1.1: Type: text/plain, Size: 1382 bytes --]
At 2021-04-22 23:56:20 -0400, Stefan Monnier wrote:
> [...] most faces that don't use colors: the *scratch* buffer's
> comments are displayed in black+bold+italic, the mode-line has a black
> background. A call of the form `(error "foo")` has foo display in
> black+italic and the `error symbol is displayed in reverse video
> (black background).
At 2021-05-12 22:29:47 +0200, Lars Ingebrigtsen wrote:
> I tried the patch, too, and I'm seeing the same -- most faces are
> different than they used to be, which is surely not what's supposed to
> happen.
But another experiment you could try: create a new frame. And another.
OK, I'll spoil the surprise --- this misbehaviour occurs *only* on the
initial frame. I don't precisely know _why_ that happens, but I do know
how it broke: I tried (and thought I had succeeded) fixing a bug that
occurred only when reverse video was enabled, which would stop creation
of new frames entirely.
> Jashank, have to continued working on this patch?
Yes --- I have a patched patch which fixes that behaviour.
The 10th revision of that patch is attached: I have been running it for
a few weeks atop 7c901d90e620b4d3651b86c13faf1e81eeb3db10 (master at the
time), so I can tell you it also works with native-compile.
I have rebased onto ec574a72f7198d9793b466f33382fff397ac4ce1 (master as
of now) and will test that.
Cheers,
~jashank
[-- Attachment #1.2: 0001-Store-frame-faces-in-hash-tables-instead-of-alists-V10.patch --]
[-- Type: text/x-patch, Size: 19188 bytes --]
From 04b8f2d131824fa9dc2bece19f4a719f977e4f4c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Pit-Claudel?= <clement.pitclaudel@live.com>
Date: Tue, 12 May 2020 21:48:32 -0400
Subject: [PATCH] Store frame faces in hash tables instead of alists
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* src/frame.h (struct frame): Add face_hash_table, remove face_alist.
(fset_face_hash_table): New function.
(fset_face_alist): Remove.
* src/frame.c (make_frame): Initialize f->face_hash_table.
(Fmake_terminal_frame): Update to work with hash tables instead of
alists.
* src/xfaces.c (lface_from_face_name_no_resolve):
(Finternal_make_lisp_face):
(update_face_from_frame_parameter): Update to work with hash tables
instead of alists.
(Fframe_face_hash_table): New function.
(Fframe_face_alist): Move to faces.el as frame-face-alist.
(syms_of_xfaces): Add frame_face_hash_table.
* lisp/progmodes/elisp-mode.el (elisp--eval-defun-1):
* lisp/frame.el (frame-set-background-mode): Update to work with hash
tables instead of alists.
* lisp/faces.el (face-new-frame-defaults): Mark obsolete.
(face-list): Update to use face--new-frame-defaults.
(frame-face-alist): Moved here from src/xfaces.c.
(x-create-frame-with-faces): Update to handle subtle semantic change
to how frame faces propagate, which otherwise breaks frame creation
with reverse video enabled.
;; Patch history:
;;
;; V1 (2020-05-12 22:41:24 -0400), (Bug#41200) message 29.
;; Authored-by: Clément Pit-Claudel <clement.pitclaudel <at> live.com>
;;
;; V2 (2020-05-13 11:13:27 -0400), (Bug#41200) message 38.
;; Fixes compilation error.
;; Reported-by: martin rudalics <rudalics <at> gmx.at>
;; Authored-by: Clément Pit-Claudel <clement.pitclaudel <at> live.com>
;;
;; V3 (2020-05-15 10:59:36 -0400), (Bug#41200) message 55.
;; Amendments to resolve issues raised or resolved by EliZ.
;; * Code-style cleanup.
;; * Rename `face_hash' to `face_map'.
;; Suggested-by: Eli Zaretskii <eliz <at> gnu.org>
;; Authored-by: Clément Pit-Claudel <clement.pitclaudel <at> live.com>
;;
;; V4 (2020-05-15 14:50:17 -0400), (Bug#41200) message 70.
;; * Settle on name `face_hash_table'.
;; * Rename `face-new-frame-defaults' to `face--new-frame-defaults'.
;; * Allow multiple variants of a face to exist.
;; Suggested-by: Eli Zaretskii <eliz <at> gnu.org>
;; Authored-by: Clément Pit-Claudel <clement.pitclaudel <at> live.com>
;;
;; V5 (2020-05-15 14:50:17 -0400), (Bug#41200) message 85.
;; Lower the size of `face--new-frame-defaults'.
;; Suggested-by: Eli Zaretskii <eliz <at> gnu.org>
;; Authored-by: Clément Pit-Claudel <clement.pitclaudel <at> live.com>
;;
;; V6 (2021-04-05 17:10:55 +1000).
;; Rebase onto master; move hunks around.
;; * Changes to `edebug.el' dropped.
;; Authored-by: Jashank Jeremy <jashank <at> rulingia.com.au>
;;
;; V7 (2021-04-06 12:31:55 +1000).
;; Fix `frame-face-alist' to cope with changes from V4.
;; Authored-by: Jashank Jeremy <jashank <at> rulingia.com.au>
;;
;; V8 (2021-04-06 13:13:33 +1000).
;; There is a subtle change to face propagation semantics at face
;; creation time, which stops face frame creation working at all
;; only if reverse video is enabled.
;; Authored-by: Jashank Jeremy <jashank <at> rulingia.com.au>
;;
;; V9 (2021-04-06 16:17:53 +1000).
;; Correct argument order of `remhash' in `elisp-mode.el'.
;; Reported-by: Benson Chu <bensonchu457 <at> gmail.com>
;; Authored-by: Jashank Jeremy <jashank <at> rulingia.com.au>
;;
;; V10 (2021-04-30 12:11:19 +1200).
;; Fix broken faces on initial frame (from prior reverse video fix).
;; Reported-by: Stefan Monnier <monnier <at> iro.umontreal.ca>
;; Authored-by: Jashank Jeremy <jashank <at> rulingia.com.au>
---
lisp/custom.el | 2 +-
lisp/faces.el | 27 ++++++++++--
lisp/frame.el | 2 +-
lisp/progmodes/elisp-mode.el | 3 +-
src/frame.c | 20 ++++++---
src/frame.h | 8 ++--
src/xfaces.c | 83 +++++++++++++++++++-----------------
7 files changed, 87 insertions(+), 58 deletions(-)
diff --git a/lisp/custom.el b/lisp/custom.el
index 078e3a8cf8..feac99d11b 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -926,7 +926,7 @@ custom-push-theme
;; the value to a fake theme, `changed'. If the theme is
;; later disabled, we use this to bring back the old value.
;;
- ;; For faces, we just use `face-new-frame-defaults' to
+ ;; For faces, we just use `face--new-frame-defaults' to
;; recompute when the theme is disabled.
(when (and (eq prop 'theme-value)
(boundp symbol))
diff --git a/lisp/faces.el b/lisp/faces.el
index 9969140f0c..bf24a1d7da 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -176,10 +176,29 @@ face-font-registry-alternatives
;;; Creation, copying.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+(make-obsolete-variable
+ 'face-new-frame-defaults
+ "use `face--new-frame-defaults' or `face-alist' instead."
+ "28.1")
+
+(defun frame-face-alist (&optional frame)
+ "Return an alist of frame-local faces defined on FRAME.
+This alist is a copy of the contents of `frame--face-hash-table'.
+For internal use only."
+ (declare (obsolete frame--face-hash-table "28.1"))
+ (let ((faces))
+ (maphash (lambda (face spec)
+ (let ((face-id (car (gethash face face--new-frame-defaults))))
+ (push `(,face-id ,face . ,spec) faces)))
+ (frame--face-hash-table frame))
+ (mapcar #'cdr (sort faces (lambda (f1 f2) (< (car f1) (car f2)))))))
(defun face-list ()
"Return a list of all defined faces."
- (mapcar #'car face-new-frame-defaults))
+ (let ((faces))
+ (maphash (lambda (face spec) (push `(,(car spec) . ,face) faces))
+ face--new-frame-defaults)
+ (mapcar #'cdr (sort faces (lambda (f1 f2) (< (car f1) (car f2)))))))
(defun make-face (face)
"Define a new face with name FACE, a symbol.
@@ -2115,6 +2134,8 @@ x-create-frame-with-faces
(unwind-protect
(progn
(x-setup-function-keys frame)
+ (dolist (face (nreverse (face-list)))
+ (face-spec-recalc face frame))
(x-handle-reverse-video frame parameters)
(frame-set-background-mode frame t)
(face-set-after-frame-default frame parameters)
@@ -2145,7 +2166,7 @@ x-create-frame-with-faces
(defun face-set-after-frame-default (frame &optional parameters)
"Initialize the frame-local faces of FRAME.
Calculate the face definitions using the face specs, custom theme
-settings, X resources, and `face-new-frame-defaults'.
+settings, X resources, and `face--new-frame-defaults'.
Finally, apply any relevant face attributes found amongst the
frame parameters in PARAMETERS."
;; The `reverse' is so that `default' goes first.
@@ -2154,7 +2175,7 @@ face-set-after-frame-default
(progn
;; Initialize faces from face spec and custom theme.
(face-spec-recalc face frame)
- ;; Apply attributes specified by face-new-frame-defaults
+ ;; Apply attributes specified by face--new-frame-defaults
(internal-merge-in-global-face face frame))
;; Don't let invalid specs prevent frame creation.
(error nil)))
diff --git a/lisp/frame.el b/lisp/frame.el
index aff1d479ec..94e0cf3c6a 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -1231,7 +1231,7 @@ frame-set-background-mode
;; during startup with -rv on the command
;; line for the initial frame, because frames
;; are not recorded in the pdump file.
- (assq face (frame-face-alist frame))
+ (gethash face (frame--face-hash-table))
(face-spec-match-p face
(face-user-default-spec face)
frame)))
diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index a56c7093e7..7ed2d3d08c 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -1325,8 +1325,7 @@ elisp--eval-defun-1
((eq (car form) 'custom-declare-face)
;; Reset the face.
(let ((face-symbol (eval (nth 1 form) lexical-binding)))
- (setq face-new-frame-defaults
- (assq-delete-all face-symbol face-new-frame-defaults))
+ (remhash face-symbol face--new-frame-defaults)
(put face-symbol 'face-defface-spec nil)
(put face-symbol 'face-override-spec nil))
form)
diff --git a/src/frame.c b/src/frame.c
index e3d65dd28f..1b7a712518 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -1017,6 +1017,10 @@ make_frame (bool mini_p)
rw->total_lines = FRAME_LINES (f) - (mini_p ? 1 : 0);
rw->pixel_height = rw->total_lines * FRAME_LINE_HEIGHT (f);
+ fset_face_hash_table
+ (f, make_hash_table (hashtest_eq, DEFAULT_HASH_SIZE, DEFAULT_REHASH_SIZE,
+ DEFAULT_REHASH_THRESHOLD, Qnil, false));
+
if (mini_p)
{
mw->top_line = rw->total_lines;
@@ -1325,7 +1329,7 @@ DEFUN ("make-terminal-frame", Fmake_terminal_frame, Smake_terminal_frame,
{
struct frame *f;
struct terminal *t = NULL;
- Lisp_Object frame, tem;
+ Lisp_Object frame;
struct frame *sf = SELECTED_FRAME ();
#ifdef MSDOS
@@ -1407,14 +1411,16 @@ DEFUN ("make-terminal-frame", Fmake_terminal_frame, Smake_terminal_frame,
store_in_alist (&parms, Qminibuffer, Qt);
Fmodify_frame_parameters (frame, parms);
- /* Make the frame face alist be frame-specific, so that each
+ /* Make the frame face hash be frame-specific, so that each
frame could change its face definitions independently. */
- fset_face_alist (f, Fcopy_alist (sf->face_alist));
- /* Simple Fcopy_alist isn't enough, because we need the contents of
- the vectors which are the CDRs of associations in face_alist to
+ fset_face_hash_table (f, Fcopy_hash_table (sf->face_hash_table));
+ /* Simple copy_hash_table isn't enough, because we need the contents of
+ the vectors which are the values in face_hash_table to
be copied as well. */
- for (tem = f->face_alist; CONSP (tem); tem = XCDR (tem))
- XSETCDR (XCAR (tem), Fcopy_sequence (XCDR (XCAR (tem))));
+ ptrdiff_t idx = 0;
+ struct Lisp_Hash_Table *table = XHASH_TABLE (f->face_hash_table);
+ for (idx = 0; idx < table->count; ++idx)
+ set_hash_value_slot (table, idx, Fcopy_sequence (HASH_VALUE (table, idx)));
f->can_set_window_size = true;
f->after_make_frame = true;
diff --git a/src/frame.h b/src/frame.h
index 75a0b184c1..e70f418dc9 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -158,8 +158,8 @@ #define EMACS_FRAME_H
There are four additional elements of nil at the end, to terminate. */
Lisp_Object menu_bar_items;
- /* Alist of elements (FACE-NAME . FACE-VECTOR-DATA). */
- Lisp_Object face_alist;
+ /* Hash table of FACE-NAME keys and FACE-VECTOR-DATA values. */
+ Lisp_Object face_hash_table;
/* A vector that records the entire structure of this frame's menu bar.
For the format of the data, see extensive comments in xmenu.c.
@@ -667,9 +667,9 @@ fset_condemned_scroll_bars (struct frame *f, Lisp_Object val)
f->condemned_scroll_bars = val;
}
INLINE void
-fset_face_alist (struct frame *f, Lisp_Object val)
+fset_face_hash_table (struct frame *f, Lisp_Object val)
{
- f->face_alist = val;
+ f->face_hash_table = val;
}
#if defined (HAVE_WINDOW_SYSTEM)
INLINE void
diff --git a/src/xfaces.c b/src/xfaces.c
index ab4440f46a..d86adef62c 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -95,9 +95,10 @@ Copyright (C) 1993-1994, 1998-2021 Free Software Foundation, Inc.
with the symbol `face' in slot 0, and a slot for each of the face
attributes mentioned above.
- There is also a global face alist `Vface_new_frame_defaults'. Face
- definitions from this list are used to initialize faces of newly
- created frames.
+ There is also a global face map `Vface_new_frame_defaults',
+ containing conses of (FACE_ID . FACE_DEFINITION). Face definitions
+ from this table are used to initialize faces of newly created
+ frames.
A face doesn't have to specify all attributes. Those not specified
have a value of `unspecified'. Faces specifying all attributes but
@@ -1962,13 +1963,11 @@ lface_from_face_name_no_resolve (struct frame *f, Lisp_Object face_name,
Lisp_Object lface;
if (f)
- lface = assq_no_quit (face_name, f->face_alist);
+ lface = Fgethash (face_name, f->face_hash_table, Qnil);
else
- lface = assq_no_quit (face_name, Vface_new_frame_defaults);
+ lface = CDR (Fgethash (face_name, Vface_new_frame_defaults, Qnil));
- if (CONSP (lface))
- lface = XCDR (lface);
- else if (signal_p)
+ if (signal_p && NILP (lface))
signal_error ("Invalid face", face_name);
check_lface (lface);
@@ -2867,11 +2866,6 @@ DEFUN ("internal-make-lisp-face", Finternal_make_lisp_face,
/* Add a global definition if there is none. */
if (NILP (global_lface))
{
- global_lface = make_vector (LFACE_VECTOR_SIZE, Qunspecified);
- ASET (global_lface, 0, Qface);
- Vface_new_frame_defaults = Fcons (Fcons (face, global_lface),
- Vface_new_frame_defaults);
-
/* Assign the new Lisp face a unique ID. The mapping from Lisp
face id to Lisp face is given by the vector lface_id_to_name.
The mapping from Lisp face to Lisp face id is given by the
@@ -2881,9 +2875,14 @@ DEFUN ("internal-make-lisp-face", Finternal_make_lisp_face,
xpalloc (lface_id_to_name, &lface_id_to_name_size, 1, MAX_FACE_ID,
sizeof *lface_id_to_name);
+ Lisp_Object face_id = make_fixnum (next_lface_id);
lface_id_to_name[next_lface_id] = face;
- Fput (face, Qface, make_fixnum (next_lface_id));
+ Fput (face, Qface, face_id);
++next_lface_id;
+
+ global_lface = make_vector (LFACE_VECTOR_SIZE, Qunspecified);
+ ASET (global_lface, 0, Qface);
+ Fputhash (face, Fcons (face_id, global_lface), Vface_new_frame_defaults);
}
else if (f == NULL)
for (i = 1; i < LFACE_VECTOR_SIZE; ++i)
@@ -2896,7 +2895,7 @@ DEFUN ("internal-make-lisp-face", Finternal_make_lisp_face,
{
lface = make_vector (LFACE_VECTOR_SIZE, Qunspecified);
ASET (lface, 0, Qface);
- fset_face_alist (f, Fcons (Fcons (face, lface), f->face_alist));
+ Fputhash (face, lface, f->face_hash_table);
}
else
for (i = 1; i < LFACE_VECTOR_SIZE; ++i)
@@ -3057,7 +3056,7 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
f = NULL;
lface = lface_from_face_name (NULL, face, true);
- /* When updating face-new-frame-defaults, we put :ignore-defface
+ /* When updating face--new-frame-defaults, we put :ignore-defface
where the caller wants `unspecified'. This forces the frame
defaults to ignore the defface value. Otherwise, the defface
will take effect, which is generally not what is intended.
@@ -3642,7 +3641,7 @@ update_face_from_frame_parameter (struct frame *f, Lisp_Object param,
/* If there are no faces yet, give up. This is the case when called
from Fx_create_frame, and we do the necessary things later in
face-set-after-frame-defaults. */
- if (NILP (f->face_alist))
+ if (XFIXNAT (Fhash_table_count (f->face_hash_table)) == 0)
return;
if (EQ (param, Qforeground_color))
@@ -4308,14 +4307,13 @@ DEFUN ("internal-lisp-face-empty-p", Finternal_lisp_face_empty_p,
return i == LFACE_VECTOR_SIZE ? Qt : Qnil;
}
-
-DEFUN ("frame-face-alist", Fframe_face_alist, Sframe_face_alist,
+DEFUN ("frame--face-hash-table", Fframe_face_hash_table, Sframe_face_hash_table,
0, 1, 0,
- doc: /* Return an alist of frame-local faces defined on FRAME.
+ doc: /* Return a hash table of frame-local faces defined on FRAME.
For internal use only. */)
(Lisp_Object frame)
{
- return decode_live_frame (frame)->face_alist;
+ return decode_live_frame (frame)->face_hash_table;
}
@@ -6832,30 +6830,32 @@ DEFUN ("show-face-resources", Fshow_face_resources, Sshow_face_resources,
#ifdef HAVE_PDUMPER
/* All the faces defined during loadup are recorded in
- face-new-frame-defaults, with the last face first in the list. We
- need to set next_lface_id to the next face ID number, so that any
- new faces defined in this session will have face IDs different from
- those defined during loadup. We also need to set up the
- lface_id_to_name[] array for the faces that were defined during
- loadup. */
+ face-new-frame-defaults. We need to set next_lface_id to the next
+ face ID number, so that any new faces defined in this session will
+ have face IDs different from those defined during loadup. We also
+ need to set up the lface_id_to_name[] array for the faces that were
+ defined during loadup. */
void
init_xfaces (void)
{
- if (CONSP (Vface_new_frame_defaults))
+ int nfaces = XFIXNAT (Fhash_table_count (Vface_new_frame_defaults));
+ if (nfaces > 0)
{
/* Allocate the lface_id_to_name[] array. */
- lface_id_to_name_size = next_lface_id =
- XFIXNAT (Flength (Vface_new_frame_defaults));
+ lface_id_to_name_size = next_lface_id = nfaces;
lface_id_to_name = xnmalloc (next_lface_id, sizeof *lface_id_to_name);
/* Store the faces. */
- Lisp_Object tail;
- int i = next_lface_id - 1;
- for (tail = Vface_new_frame_defaults; CONSP (tail); tail = XCDR (tail))
+ struct Lisp_Hash_Table* table = XHASH_TABLE (Vface_new_frame_defaults);
+ for (ptrdiff_t idx = 0; idx < nfaces; ++idx)
{
- Lisp_Object lface = XCAR (tail);
- eassert (i >= 0);
- lface_id_to_name[i--] = XCAR (lface);
+ Lisp_Object lface = HASH_KEY (table, idx);
+ Lisp_Object face_id = CAR (HASH_VALUE (table, idx));
+ if (FIXNATP (face_id)) {
+ int id = XFIXNAT (face_id);
+ eassert (id >= 0);
+ lface_id_to_name[id] = lface;
+ }
}
}
face_attr_sym[0] = Qface;
@@ -7011,7 +7011,7 @@ syms_of_xfaces (void)
defsubr (&Sinternal_copy_lisp_face);
defsubr (&Sinternal_merge_in_global_face);
defsubr (&Sface_font);
- defsubr (&Sframe_face_alist);
+ defsubr (&Sframe_face_hash_table);
defsubr (&Sdisplay_supports_face_attributes_p);
defsubr (&Scolor_distance);
defsubr (&Sinternal_set_font_selection_order);
@@ -7035,9 +7035,12 @@ syms_of_xfaces (void)
the "specifity" of a face specification and should be let-bound
only for this purpose. */);
- DEFVAR_LISP ("face-new-frame-defaults", Vface_new_frame_defaults,
- doc: /* List of global face definitions (for internal use only.) */);
- Vface_new_frame_defaults = Qnil;
+ DEFVAR_LISP ("face--new-frame-defaults", Vface_new_frame_defaults,
+ doc: /* Hash table of global face definitions (for internal use only.) */);
+ Vface_new_frame_defaults =
+ /* 33 entries is enough to fit all basic faces */
+ make_hash_table (hashtest_eq, 33, DEFAULT_REHASH_SIZE,
+ DEFAULT_REHASH_THRESHOLD, Qnil, false);
DEFVAR_LISP ("face-default-stipple", Vface_default_stipple,
doc: /* Default stipple pattern used on monochrome displays.
--
2.31.1
[-- Attachment #2: OpenPGP Digital Signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2021-05-13 3:56 ` Jashank Jeremy
@ 2021-05-13 9:15 ` Lars Ingebrigtsen
2021-05-13 23:26 ` Jashank Jeremy
2021-07-21 14:02 ` Lars Ingebrigtsen
1 sibling, 1 reply; 57+ messages in thread
From: Lars Ingebrigtsen @ 2021-05-13 9:15 UTC (permalink / raw)
To: Jashank Jeremy; +Cc: Emacs bug41200, Clément Pit-Claudel, Stefan Monnier
Jashank Jeremy <jashank@rulingia.com.au> writes:
> The 10th revision of that patch is attached: I have been running it for
> a few weeks atop 7c901d90e620b4d3651b86c13faf1e81eeb3db10 (master at the
> time), so I can tell you it also works with native-compile.
I can confirm that this version fixes the face problems observed with
the previous version.
I don't see your copyright assignment papers on file -- is that in the
pipeline, or have you yet to start the assignment process?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2021-05-13 9:15 ` Lars Ingebrigtsen
@ 2021-05-13 23:26 ` Jashank Jeremy
2021-06-12 12:15 ` Lars Ingebrigtsen
0 siblings, 1 reply; 57+ messages in thread
From: Jashank Jeremy @ 2021-05-13 23:26 UTC (permalink / raw)
To: Lars Ingebrigtsen
Cc: Emacs bug41200, Clément Pit-Claudel, Stefan Monnier
[-- Attachment #1: Type: text/plain, Size: 659 bytes --]
At 2021-05-13 11:15:29 +0200, Lars Ingebrigtsen wrote:
> Jashank Jeremy <jashank@rulingia.com.au> writes:
>> The 10th revision of that patch is attached: I have been running it for
>> a few weeks atop 7c901d90e620b4d3651b86c13faf1e81eeb3db10 (master at the
>> time), so I can tell you it also works with native-compile.
>
> I can confirm that this version fixes the face problems observed with
> the previous version.
Excellent, thanks.
> I don't see your copyright assignment papers on file -- is that in the
> pipeline, or have you yet to start the assignment process?
Started ... but presently stalled due to the employer pipeline hazard.
~jashank
[-- Attachment #2: OpenPGP Digital Signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2021-05-13 23:26 ` Jashank Jeremy
@ 2021-06-12 12:15 ` Lars Ingebrigtsen
2021-06-13 3:19 ` Richard Stallman
2021-07-06 12:41 ` Aaron Jensen
0 siblings, 2 replies; 57+ messages in thread
From: Lars Ingebrigtsen @ 2021-06-12 12:15 UTC (permalink / raw)
To: Jashank Jeremy; +Cc: Emacs bug41200, Clément Pit-Claudel, Stefan Monnier
Jashank Jeremy <jashank@rulingia.com.au> writes:
>> I don't see your copyright assignment papers on file -- is that in the
>> pipeline, or have you yet to start the assignment process?
>
> Started ... but presently stalled due to the employer pipeline hazard.
This was a month ago -- has there been any further progress here?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2021-06-12 12:15 ` Lars Ingebrigtsen
@ 2021-06-13 3:19 ` Richard Stallman
2021-07-06 12:41 ` Aaron Jensen
1 sibling, 0 replies; 57+ messages in thread
From: Richard Stallman @ 2021-06-13 3:19 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: jashank, 41200, cpitclaudel, monnier
[[[ To any NSA and FBI agents reading my email: please consider ]]]
[[[ whether defending the US Constitution against all enemies, ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]
> >> I don't see your copyright assignment papers on file -- is that in the
> >> pipeline, or have you yet to start the assignment process?
> >
> > Started ... but presently stalled due to the employer pipeline hazard.
> This was a month ago -- has there been any further progress here?
Talking with people at the employer, and with the FSF staff, and
encouraging them to talk with each other, could enable them to unblock
it. Some companies' lawyers look at the issue in a very narrow way,
and the FSF staff could help them see a solution.
--
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2021-06-12 12:15 ` Lars Ingebrigtsen
2021-06-13 3:19 ` Richard Stallman
@ 2021-07-06 12:41 ` Aaron Jensen
1 sibling, 0 replies; 57+ messages in thread
From: Aaron Jensen @ 2021-07-06 12:41 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: jashank, 41200, cpitclaudel, monnier
Lars Ingebrigtsen <larsi@gnus.org> writes:
> Jashank Jeremy <jashank@rulingia.com.au> writes:
>
>>> I don't see your copyright assignment papers on file -- is that in the
>>> pipeline, or have you yet to start the assignment process?
>>
>> Started ... but presently stalled due to the employer pipeline hazard.
>
>This was a month ago -- has there been any further progress here?
Is there any chance that Clément could pick this up from his original
patch and avoid waiting for copyright assignment of the
corrected/up-to-date patch? I'd do it mysel if I were more familiar with
the code, but I'm not.
Aaron
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2021-05-13 3:56 ` Jashank Jeremy
2021-05-13 9:15 ` Lars Ingebrigtsen
@ 2021-07-21 14:02 ` Lars Ingebrigtsen
2021-07-21 14:28 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
1 sibling, 1 reply; 57+ messages in thread
From: Lars Ingebrigtsen @ 2021-07-21 14:02 UTC (permalink / raw)
To: Jashank Jeremy; +Cc: Emacs bug41200, Clément Pit-Claudel, Stefan Monnier
Jashank Jeremy <jashank@rulingia.com.au> writes:
> Yes --- I have a patched patch which fixes that behaviour.
>
> The 10th revision of that patch is attached: I have been running it for
> a few weeks atop 7c901d90e620b4d3651b86c13faf1e81eeb3db10 (master at the
> time), so I can tell you it also works with native-compile.
>
> I have rebased onto ec574a72f7198d9793b466f33382fff397ac4ce1 (master as
> of now) and will test that.
I see that the copyright assignment paperwork was finished a couple of
weeks ago, so I've now re-tested your patch, and I can't see any
glitches, so I've pushed it to Emacs 28.
Thanks!
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2021-07-21 14:02 ` Lars Ingebrigtsen
@ 2021-07-21 14:28 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-07-21 14:32 ` Clément Pit-Claudel
0 siblings, 1 reply; 57+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-07-21 14:28 UTC (permalink / raw)
To: Lars Ingebrigtsen
Cc: Jashank Jeremy, Emacs bug41200, Clément Pit-Claudel
Lars Ingebrigtsen [2021-07-21 16:02:12] wrote:
> I see that the copyright assignment paperwork was finished a couple of
> weeks ago, so I've now re-tested your patch, and I can't see any
> glitches, so I've pushed it to Emacs 28.
Yay!
Stefan
^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined
2021-07-21 14:28 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-07-21 14:32 ` Clément Pit-Claudel
0 siblings, 0 replies; 57+ messages in thread
From: Clément Pit-Claudel @ 2021-07-21 14:32 UTC (permalink / raw)
To: Stefan Monnier, Lars Ingebrigtsen; +Cc: Jashank Jeremy, Emacs bug41200
On 7/21/21 10:28 AM, Stefan Monnier wrote:
> Lars Ingebrigtsen [2021-07-21 16:02:12] wrote:
>> I see that the copyright assignment paperwork was finished a couple of
>> weeks ago, so I've now re-tested your patch, and I can't see any
>> glitches, so I've pushed it to Emacs 28.
Thanks everyone!
Now to figure why tooltips are blinking so much lately ^^ (it's from before this patch)
^ permalink raw reply [flat|nested] 57+ messages in thread
end of thread, other threads:[~2021-07-21 14:32 UTC | newest]
Thread overview: 57+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-12 4:30 bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined Clément Pit-Claudel
2020-05-12 6:42 ` martin rudalics
2020-05-12 11:30 ` Clément Pit-Claudel
2020-05-12 15:12 ` martin rudalics
2020-05-12 17:19 ` Clément Pit-Claudel
2020-05-12 17:42 ` martin rudalics
2020-05-12 17:58 ` Eli Zaretskii
2020-05-13 14:58 ` martin rudalics
2020-05-12 15:27 ` Eli Zaretskii
2020-05-13 2:41 ` Clément Pit-Claudel
2020-05-13 14:58 ` martin rudalics
2020-05-13 15:13 ` Clément Pit-Claudel
2020-05-13 17:42 ` martin rudalics
2020-05-15 11:05 ` Eli Zaretskii
2020-05-15 14:59 ` Clément Pit-Claudel
2020-05-15 15:17 ` Eli Zaretskii
2020-05-15 15:33 ` Noam Postavsky
2020-05-15 16:22 ` Clément Pit-Claudel
2020-05-15 17:28 ` Eli Zaretskii
2020-05-15 18:50 ` Clément Pit-Claudel
2020-05-15 19:05 ` Eli Zaretskii
2020-05-15 19:23 ` Clément Pit-Claudel
2020-05-15 19:38 ` Eli Zaretskii
2020-05-15 19:52 ` Clément Pit-Claudel
2020-05-16 23:03 ` Juri Linkov
2020-05-16 23:43 ` Clément Pit-Claudel
2020-05-17 21:59 ` Juri Linkov
2020-05-18 1:19 ` Clément Pit-Claudel
2020-05-19 21:48 ` Juri Linkov
[not found] ` <83a71z135p.fsf@gnu.org>
2020-05-23 22:47 ` Juri Linkov
2020-05-24 2:33 ` Eli Zaretskii
2020-05-24 21:50 ` Juri Linkov
2020-06-08 0:21 ` Juri Linkov
2020-06-20 7:47 ` Eli Zaretskii
2020-06-20 16:55 ` Clément Pit-Claudel
2020-07-04 7:58 ` Eli Zaretskii
2020-09-13 2:53 ` Benson Chu
2020-05-15 14:03 ` Stefan Monnier
2020-05-15 14:34 ` Eli Zaretskii
2020-05-15 19:10 ` Clément Pit-Claudel
2020-05-15 21:23 ` Stefan Monnier
2020-05-16 8:45 ` martin rudalics
2021-04-06 6:35 ` Jashank Jeremy
2021-04-06 12:30 ` Eli Zaretskii
2021-04-06 15:07 ` Clément Pit-Claudel
2021-04-06 15:50 ` Eli Zaretskii
2021-04-23 3:56 ` Stefan Monnier
2021-05-12 20:29 ` Lars Ingebrigtsen
2021-05-13 3:56 ` Jashank Jeremy
2021-05-13 9:15 ` Lars Ingebrigtsen
2021-05-13 23:26 ` Jashank Jeremy
2021-06-12 12:15 ` Lars Ingebrigtsen
2021-06-13 3:19 ` Richard Stallman
2021-07-06 12:41 ` Aaron Jensen
2021-07-21 14:02 ` Lars Ingebrigtsen
2021-07-21 14:28 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-07-21 14:32 ` Clément Pit-Claudel
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).