Subject: Re: bug#70216: 30.0.50; self-insert-command doesn't respect create-lockfiles

From: Theodor Thornhill <[email-masked]>
Date: Sat, 6 Apr 2024 16:37:11 -0600

Eli Zaretskii <[email-masked]> writes:

>> Date: Fri, 05 Apr 2024 12:52:39 +0200
>> From:  Theodor Thornhill via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <[email-masked]>
>> 
>> I'm digging more and more into the internals of Emacs these days, and
>> while tracing input lag performance i noticed that we don't respect the
>> create-lockfiles option, at least not fully.
>> 
>> The documentation states:
>> ```
>> If the option `create-lockfiles' is nil, this does nothing.
>> ```
>
> I guess we need to fix the documentation, then.
>

Sure ;)

>> However, the nothing is postponed until later, inside the lock_file
>> function. This means we won't cretae the lockfile, but we will get the
>> truename-buffer, verify visited file, etc, on my system amounting to
>> some time spent which I thought I opted out of.
>
> This is on purpose.  We decided that create-lockfiles = nil is meant
> to disable the creation of lock files, but it is not meant to disable
> userlock--ask-user-about-supersession-threat, for example, which is an
> important feature we don't want to lose just because the user doesn't
> want to have lock files.  See bug#53207 for some discussion of this.
> Bug#49507 is also relevant.
>

I understand. I guess that does make sense.

>> Along with a patch that fixes this behavior I've added 4 screenshots of
>> graphs taken through intel_pt tracing on trigger of self-insert inside a
>> c file.
>
> I don't think we want to install this patch, since we do want the
> file-change detection, which is done as part of this code.
>

Sure - though I think we should investigate a bit further on what we are
doing inside of this code path.

>> The lock file related code can take up to a millisecond on my system,
>> but is usually in the range of 200-300 microseconds.
>
> Lock files and file-change detection are Emacs safety measures that
> are important on any modern OS.  Disabling them because they eat up
> CPU cycles is wrong, from where I stand.  That said, hundreds of
> milliseconds for 2 calls to 'stat' sounds excessive to me, so please
> tell more details and try to show the breakdown of this long time.
>

Sure! So it all starts in prepare_to_modify_buffer, where we run the
verify_internal_modification. This is pretty fast, in the order of ~10
microseconds. Next up is the Flock_file, which delegates to
lock_file. Before jumping into lock_file we run Ffind_file_name_handler,
which takes between 50 and 200 microseconds on my system, depending on
general load, I assume. now we jump into lock_file, from which we inside
Fverify_visited_file_modtime run Ffind_file_name_handler for good
measure, for another 50-200microseconds.

This to me looks like _at least_ one too many calls to
Ffind_file_name_handler causing almost half a millisecond on keypress,
which sounds very excessive. 

>> It seems there was some cleanup/changes around this behavior which i
>> believe caused this as a regression, from around 3 years ago. I _think_
>> this commit: 9ce6541ac9710933beca7f9944087fe4849d5ae9
>
> There were more changes there, see above.

I'll read this more closely

If you're interested, take this trace file and plop it into
https://magic-trace.org and search for self_insert_command and move
around with wasd.

Thanks,
Theo