GNU bug report logs - #70216
30.0.50; self-insert-command doesn't respect create-lockfiles

Previous Next

To reply to this bug, email your comments to 70216 AT debbugs.gnu.org.
There is no need to reopen the bug first.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; self-insert-command doesn't respect create-lockfiles
Date: Fri, 05 Apr 2024 12:52:39 +0200

[Message part 1 (text/plain, inline)]

Hi all!

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.
```
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.

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.

to reproduce what I do, from inside my emacs source code folder:

```
;; normal emacs built from source a couple of days ago
emacs -Q +644 src/filelock.c
emacs -Q -eval "(setq create-lockfiles nil)" +644 src/filelock.c

;; emacs with the provided patch
./src/emacs -Q +644 src/filelock.c
./src/emacs -Q -eval "(setq create-lockfiles nil)" +644 src/filelock.c
```

The first two screenshots show the current behavior without this patch,
where you can see that we still do some lock-file related computing even
though the setting is off. The last two show that we don't do it at all,
while preserving the vanilla operation when create-lockfiles = t.

The lock file related code can take up to a millisecond on my system,
but is usually in the range of 200-300 microseconds.

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

Theo

[emacs-Q.png (image/png, attachment)]

[emacs-Q-create-lockfiles-nil.png (image/png, attachment)]

[patched-emacs-Q.png (image/png, attachment)]

[patched-emacs-Q-create-lockfiles-nil.png (image/png, attachment)]

[0001-Actually-do-nothing-when-create-lockfiles-is-nil.patch (text/x-diff, attachment)]

Message #8 received at 70216 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 70216 <at> debbugs.gnu.org
Subject: Re: bug#70216: 30.0.50;
 self-insert-command doesn't respect create-lockfiles
Date: Fri, 05 Apr 2024 16:03:31 +0300

> 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" <bug-gnu-emacs <at> gnu.org>
> 
> 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.

> 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.

> 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.

> 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.

> 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.

Message #9 received at 70216-quiet <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: 70216-quiet <at> debbugs.gnu.org
Subject: Re: bug#70216: 30.0.50; self-insert-command doesn't respect
 create-lockfiles
Date: Sat, 6 Apr 2024 16:37:11 -0600

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> 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" <bug-gnu-emacs <at> gnu.org>
>> 
>> 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

[trace.fxt.gz (application/gzip, attachment)]

Message #12 received at 70216 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 70216 <at> debbugs.gnu.org
Subject: Re: bug#70216: 30.0.50; self-insert-command doesn't respect
 create-lockfiles
Date: Sun, 07 Apr 2024 09:31:42 +0300

> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: 70216 <at> debbugs.gnu.org
> Date: Sat, 06 Apr 2024 23:29:56 +0200
> 
> >> The documentation states:
> >> ```
> >> If the option `create-lockfiles' is nil, this does nothing.
> >> ```
> >
> > I guess we need to fix the documentation, then.
> >
> 
> Sure ;)

Done on the emacs-29 branch.

> > 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. 

The extra call is only when the file is local.  And losing 50 to 200
microseconds doesn't sound bad to me.

Also, we don't call lock-file on each keystroke, only on the first one
that makes the buffer modified when it was previously unmodified.  So
when you type several characters in sequence, the net slowdown will be
very small.

In any case, you originally said this took hundreds of milliseconds,
not hundreds of microseconds.  Was that a typo, and you really meant
microseconds?

> 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.

Hmm... you just sent a 15MB file, uncompressed, to everyone who is
subscribed to bug-gnu-emacs...

Message #15 received at 70216 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 70216 <at> debbugs.gnu.org
Subject: Re: bug#70216: 30.0.50; self-insert-command doesn't respect
 create-lockfiles
Date: Sun, 07 Apr 2024 08:40:58 +0200

[Message part 1 (text/html, inline)]

Message #20 received at 70216-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 70216-done <at> debbugs.gnu.org
Subject: Re: bug#70216: 30.0.50; self-insert-command doesn't respect
 create-lockfiles
Date: Sun, 07 Apr 2024 11:11:40 +0300

> Date: Sun, 07 Apr 2024 08:40:58 +0200
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: 70216 <at> debbugs.gnu.org
> 
> I think we can close this if you don't think this is worth it, at least the docs got fixed :)

Closing.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.