Subject: bug#60691: 29.0.60; Slow tree-sitter font-lock in ruby-ts-mode

From: Yuan Fu <[email-masked]>
Date: Fri, 13 Jan 2023 19:48:40 -0800

> On Jan 13, 2023, at 3:51 AM, Eli Zaretskii <[email-masked]> wrote:
> 
>> From: Yuan Fu <[email-masked]>
>> Date: Fri, 13 Jan 2023 01:15:09 -0800
>> Cc: Dmitry Gutov <[email-masked]>,
>> [email-masked],
>> Juri Linkov <[email-masked]>,
>> Stefan Monnier <[email-masked]>
>> 
>>> On Jan 12, 2023, at 11:57 PM, Eli Zaretskii <[email-masked]> wrote:
>>> 
>>>> Cc: [email-masked], [email-masked]
>>>> Date: Fri, 13 Jan 2023 01:40:56 +0200
>>>> From: Dmitry Gutov <[email-masked]>
>>>> 
>>>> Managed to reproduce this after running the test in a couple of 
>>>> different files.
>>>> 
>>>> But 'M-x memory-usage' says no such command, and 'M-x memory-report' 
>>>> ends up with this error:
>>>> 
>>>> Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p nil)
>>>>  memory-report--gc-elem(nil strings)
>>>>  memory-report--garbage-collect()
>>>>  memory-report()
>>> 
>>> This means GC is disabled in this session at the time you invoke
>>> memory-report.  Which shouldn't happen, of course.  It sounds like
>>> your pure Lisp storage overflowed, and that disabled GC.
>>> 
>>> And I think I see the problem: we use build_pure_c_string in treesit.c
>>> in places that we shouldn't.
>>> 
>>> Yuan, build_pure_c_string should only be used in places such as
>>> syms_of_treesit, which are called just once, during dumping.  Look at
>>> all the other calls to this function in the sources, and you will see
>>> it.  In all other cases, you should do one of the following:
>>> 
>>> . for strings whose text is fixed, define a variable, give it the
>>>   value in syms_of_treesit using build_pure_c_string, then use that
>>>   variable elsewhere in the source
>> 
>> Can I define a bunch of static C variables and initialize them in syms_of_treesit, or they have to be all Lisp variables? Eg,
>> 
>> static Lisp_Object TREESIT_STAR;
>> 
>> ...
>> 
>> void
>> syms_of_treesit (void)
>> {
>> ...
>> TREESIT_STAR = build_pure_c_string ("*");
>> ...
>> }
> 
> Yes, of course.  Look, for example, how coding.c does that:
> 
>  /* A string that serves as name of the reusable work buffer, and as base
>     name of temporary work buffers used for code-conversion operations.  */
>  static Lisp_Object Vcode_conversion_workbuf_name;
>  [...]
>  void
>  syms_of_coding (void)
>  {
>  [...]
>    staticpro (&Vcode_conversion_workbuf_name);
>    Vcode_conversion_workbuf_name = build_pure_c_string (" *code-conversion-work*");
> 
> But please keep the convention of naming such variables Vsome_thing,
> both regarding the "V" and the fact that the name is otherwise
> lower-case.

Thanks, I pushed a fix for it. I also used intern_c_string in some places like these:

intern_c_string (":?”)
intern_c_string (":*")

I want to change them to use DEFSYM, but what should be the c name for them?

Yuan