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: Fri, 15 May 2020 12:22:53 -0400 Message-ID: References: <8fd8896a-cd5c-66f4-4792-f65cac4dc4f5@gmail.com> <83lflx896q.fsf@gnu.org> <837dxd31cb.fsf@gnu.org> <789d786d-a07a-65c1-c0e4-433e4c18d64e@gmail.com> <835zcxgrby.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="96572"; 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, Anders Lindgren To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri May 15 18:24:12 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 1jZd86-000Oy8-VB for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 15 May 2020 18:24:11 +0200 Original-Received: from localhost ([::1]:36760 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jZd86-0002ue-29 for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 15 May 2020 12:24:10 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:35280) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jZd7y-0002uQ-Qb for bug-gnu-emacs@gnu.org; Fri, 15 May 2020 12:24:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:55286) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jZd7y-000888-8s for bug-gnu-emacs@gnu.org; Fri, 15 May 2020 12:24:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1jZd7y-0005yS-36 for bug-gnu-emacs@gnu.org; Fri, 15 May 2020 12:24: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: Fri, 15 May 2020 16:24: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.158955978222854 (code B ref 41200); Fri, 15 May 2020 16:24:02 +0000 Original-Received: (at 41200) by debbugs.gnu.org; 15 May 2020 16:23:02 +0000 Original-Received: from localhost ([127.0.0.1]:38599 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jZd70-0005wX-DF for submit@debbugs.gnu.org; Fri, 15 May 2020 12:23:02 -0400 Original-Received: from mail-qv1-f44.google.com ([209.85.219.44]:42347) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jZd6y-0005vd-IW for 41200@debbugs.gnu.org; Fri, 15 May 2020 12:23:00 -0400 Original-Received: by mail-qv1-f44.google.com with SMTP id g20so1336850qvb.9 for <41200@debbugs.gnu.org>; Fri, 15 May 2020 09:23:00 -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:content-transfer-encoding; bh=tCygyuYx99Yf8Qa20hYMvfKBiNKYY50E6TlPDpPdG6w=; b=LmNBtlbQ7mmrP3CkH7dVEro4trqzhC+7I37UD7wcoEFuUuUsIDz+VbzVV9bTHCIVuR rye2Xe2AOcebqwR547aKLwlRtwgiGnHi0xPsjzXG2yvlWAGxa3Qhh6unfEk0hro28yay gxe97MqcTNYS2ykc86dFLyfB6uMaKcXd7KWWLAeSK7l5a9tfmGaB6pN9I54n3g6jnNMZ B9EOxfGtHt8Mk7UmrlG+Od88V0oMDxs8MFfVVz3UAPx40KcdzRu7gDW2WlU2OHnnO9Sl DEUnb1YN7LNgAUY2ij8Th2htEDp3W2a4kPKM6wRu4DFYRA1TbcNax4dvyrwiEUWjIWqH BlMA== 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 :content-transfer-encoding; bh=tCygyuYx99Yf8Qa20hYMvfKBiNKYY50E6TlPDpPdG6w=; b=E56VZQUu1ORaA7xy/YujRdyS5GzFr1N4iVrQCRhrvN7x9lXfNa+U1jPFHlmIsuodBF LhGa7HNrEQTHskGfZyZ1IANO1v1zw36nvxyHPWTeXNm1kO08XaU+s4wORHbaNhJYZsDg Y1l7wOfZTSkAm/AqQsi3dtx7x0AqFrSD0yF0m7LP26NPlvYMR6uh/hGqiSAgO4gVRkkL AM6hNbraKVF5RjIUtOo3wFuIboXspAZbqUpmjFs5PxMsNP/pu+kM+FA+0MSzBmUTCi5o hMBeXMF+hv9M7GeEF6nDgn6cMlgmU4uAIiopPhGTkfxiQ5RpPm4kGWSDvFMtaKZBfYoz m1sA== X-Gm-Message-State: AOAM532FmhhTlXlopXVePyDTrfY91tPcxUFnZbFdmtmQbRUH67zNiQPQ fpM8gXxHFbaquEQeAYOO4/k= X-Google-Smtp-Source: ABdhPJzI/onyMgD9nruWA+EhWsw5eLR/gzw6TwplLO2l29OlAAxXJmGbQ3ytJ7xYD19TBnkJNx4SCQ== X-Received: by 2002:ad4:4e65:: with SMTP id ec5mr4287021qvb.204.1589559774883; Fri, 15 May 2020 09:22:54 -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 k25sm1813884qki.135.2020.05.15.09.22.53 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 15 May 2020 09:22:54 -0700 (PDT) In-Reply-To: <835zcxgrby.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:180342 Archived-At: 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.