emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: How to add pseudo vector types


From: Eli Zaretskii
Subject: Re: How to add pseudo vector types
Date: Thu, 22 Jul 2021 22:05:29 +0300

> From: Yuan Fu <casouri@gmail.com>
> Date: Thu, 22 Jul 2021 13:47:20 -0400
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,
>  Clément Pit-Claudel <cpitclaudel@gmail.com>,
>  emacs-devel@gnu.org
> 
> > More generally: is the problem real?  If you make a file that is 1000
> > copies of xdisp.c, and then submit it to TS, do you really get 10GB of
> > memory consumption?  This is something that is good to know up front,
> > so we'd know what to expect down the road.
> 
> Yes. I concatenated 100 xdisp.c together, and parsed them with my simple C 
> program. It used 1.8 G. I didn’t test for 1000 together, but I think the 
> trend is linear.

That's good to know, thanks.

So what does TS do if it attempts to allocate more memory and that
fails?  Regardless, we'd need some fallback strategy, because AFAIU
many people run with VM overcommit enabled, so the OOM killer will
just kill the Emacs process when it asks for too much memory.

> >>>> +DEFUN ("tree-sitter-node-type",
> >>>> +       Ftree_sitter_node_type, Stree_sitter_node_type, 1, 1, 0,
> >>>> +       doc: /* Return the NODE's type as a symbol.  */)
> >>>> +  (Lisp_Object node)
> >>>> +{
> >>>> +  CHECK_TS_NODE (node);
> >>>> +  TSNode ts_node = XTS_NODE (node)->node;
> >>>> +  const char *type = ts_node_type(ts_node);
> >>>> +  return intern_c_string (type);
> >>> 
> >>> Why do we need to intern the string each time? can't we store the
> >>> interned symbol there, instead of a C string, in the first place?
> >> 
> >> I’m not sure what do you mean by “store the interned symbol there”, where 
> >> do I store the interned symbol?
> > 
> > In the struct that ts_node_type accesses, instead of the 'char *'
> > string you store there now.
> 
> The struct that ts_node_type accesses is a TSNode, which is defined by 
> tree-sitter. ts_node_type is an API provided by tree-sitter, I’m just 
> exposing it to lisp. I could return strings instead of symbols, but I thought 
> symbols might be more appropriate and more convenient for users of this 
> function. 

Maybe there's a better way of exposing that to Lisp.  But that's a
minor point, it can be left for later.

> Is below the correct way to set a buffer-local variable? (I’m setting 
> tree-sitter-parser-list.)
> 
> struct buffer *old_buffer = current_buffer;
>   set_buffer_internal (XBUFFER (buffer));
> 
>   Fset (Qtree_sitter_parser_list,
>       Fcons (lisp_parser, Fsymbol_value (Qtree_sitter_parser_list)));
> 
>   set_buffer_internal (old_buffer);

Yes, but it would be better to use DEFVAR_LISP and then you could
assign directly to Vtree_sitter_parser_list, instead of using Fset.

> Also, we don’t call change hooks in replace_range_2, why?

Because it is called in a loop, one character at a time.  The caller
of replace_range_2 calls these hooks for the entire region, once.

> Should I update tree-sitter trees in that function, or should I not?

The only caller is casify_region, so you could update there.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]