From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: =?UTF-8?Q?Cl=C3=A9ment?= Pit-Claudel Newsgroups: gmane.emacs.bugs Subject: bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined Date: Tue, 12 May 2020 22:41:24 -0400 Message-ID: References: <8fd8896a-cd5c-66f4-4792-f65cac4dc4f5@gmail.com> <83lflx896q.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------203724E7FFC950361B806F14" Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="45414"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 Cc: 41200@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed May 13 04:42:09 2020 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jYhLV-000Bhy-7G for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 13 May 2020 04:42:09 +0200 Original-Received: from localhost ([::1]:34818 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jYhLT-0007ej-W8 for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 12 May 2020 22:42:08 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:50106) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jYhLO-0007do-Gi for bug-gnu-emacs@gnu.org; Tue, 12 May 2020 22:42:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:45460) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jYhLO-0005DS-7M for bug-gnu-emacs@gnu.org; Tue, 12 May 2020 22:42:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1jYhLO-00027n-4x for bug-gnu-emacs@gnu.org; Tue, 12 May 2020 22:42:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: =?UTF-8?Q?Cl=C3=A9ment?= Pit-Claudel Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 13 May 2020 02:42:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 41200 X-GNU-PR-Package: emacs Original-Received: via spool by 41200-submit@debbugs.gnu.org id=B41200.15893376958129 (code B ref 41200); Wed, 13 May 2020 02:42:02 +0000 Original-Received: (at 41200) by debbugs.gnu.org; 13 May 2020 02:41:35 +0000 Original-Received: from localhost ([127.0.0.1]:57006 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jYhKw-000272-6q for submit@debbugs.gnu.org; Tue, 12 May 2020 22:41:35 -0400 Original-Received: from mail-qk1-f175.google.com ([209.85.222.175]:39341) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jYhKu-00026j-7T for 41200@debbugs.gnu.org; Tue, 12 May 2020 22:41:33 -0400 Original-Received: by mail-qk1-f175.google.com with SMTP id a136so6984529qkg.6 for <41200@debbugs.gnu.org>; Tue, 12 May 2020 19:41:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language; bh=yi61qkNXk+D9+Dvj1IhmA+DARRUctBqYF6aadcg/PeE=; b=PZ3iBCNWPMW+W5tsPC7zQFbMWyqbA0tkUcgsZPSLMlPggA4txkDnRa0LQAXB2/KFQc S83rOQl2dRaoydTml/rY5ukOc7lTcF76n/m8B+xgxP9//aVZ34WSUCWJvM68tI/TIvMb Io+MCdkiLjEX+BOeO9imUdL/VkbfqbaUu4jNz6Dm3My2/Hxb3dmDTBblQw6zMw04SNwy yc4Yf7IX/0Du/mVw0w/+ylfGrQjxNtB6MGKoUbf66DkSunvwrnpvoYBjpWtJmzq15UmA nYa0ocWskhB/dewLjNs/3E7Fuqo/q2BfJMCIlCeXaPwtQcATa5z/D4nSTWmf4llTISoI hYJw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=yi61qkNXk+D9+Dvj1IhmA+DARRUctBqYF6aadcg/PeE=; b=orw5jV7KET137JrY5IcR0gMJf50gH+Al8Gm16dw1vZrxzZrfvLg6RLgn0OxbKu6vGn Z8EKlLia6nbT/XSysWhxNN6qMI6cgl4hJXf+YPGw19U9ZUJZXVrZERGHxngH50i/h+py dC7RmVT/AB0hAhqMCAnWVNuTYVE+kQ6pBfgoh214n6G5DXRvpVu6IYSMzCX2nZYPMYBx JJGaOvXQjrgJE1ECMTr45FkRLlsuEuif8fh2tOq1lZzd4srJGRQ5t9eqz/v06RuGqmjL UMt3kW4IXeH+TnG2ArgCgG0vQNZ3PSHrA3wWyJ56lq0Uo6F10vKzANmz090uhLoJyMCt 3CKw== X-Gm-Message-State: AGi0Pub98WMi8agnSiG8XXVuKKGQLZqxixKLjMIG8yMuEGw981bOGVTa eh8ihuCNMgx+wDczBPHX+ASZnDdCmvE= X-Google-Smtp-Source: APiQypKxIKYxyN4PqAFt5yT9SKqBFarjz0yCeCcfk3GbSZEdyFC0gAqkWGk/09vSdDy25y9ush0MeA== X-Received: by 2002:a37:48c8:: with SMTP id v191mr22458155qka.268.1589337686278; Tue, 12 May 2020 19:41:26 -0700 (PDT) Original-Received: from ?IPv6:2601:184:4180:66e7:4d17:b25e:8d9:2188? ([2601:184:4180:66e7:4d17:b25e:8d9:2188]) by smtp.googlemail.com with ESMTPSA id e3sm1992203qtg.61.2020.05.12.19.41.25 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 12 May 2020 19:41:25 -0700 (PDT) In-Reply-To: <83lflx896q.fsf@gnu.org> Content-Language: en-GB X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:180141 Archived-At: This is a multi-part message in MIME format. --------------203724E7FFC950361B806F14 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit 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% - # 768 80% - funcall-interactively 768 80% - eval-defun 715 74% - apply 713 74% - # 712 74% - elisp--eval-defun 712 74% - eval-region 709 74% - apply 709 74% - # 709 74% - endless/eval-overlay 709 74% - apply 709 74% - # 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% + # 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. --------------203724E7FFC950361B806F14 Content-Type: text/x-patch; charset=UTF-8; name="0001-Store-frame-faces-in-hash-tables-instead-of-alists.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0001-Store-frame-faces-in-hash-tables-instead-of-alists.patc"; filename*1="h" >From 6d3d5d94189ae31fdbc29b2db0707d6b13a5c362 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Pit-Claudel?= 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 --------------203724E7FFC950361B806F14--