GNU bug report logs - #78340
[PATCH] New option for comp *.eln file name by the file timestamp of *.el

Previous Next

To reply to this bug, email your comments to 78340 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: Lin Sun <sunlin7 <at> hotmail.com>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: [PATCH] New option for comp *.eln file name by the file timestamp of
 *.el
Date: Sat, 10 May 2025 00:07:52 +0000

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

Hi,

Currently Emacs with native comp will comp .eln file name by the full context of the .el or .el.gz file in function "comp-el-to-eln-rel-filename".
For example, emacs startup will require the simple.elc, then have to read full of simple.el.gz to calculate the name simple-X-Y.eln. 

That's slow on Windows.

This patch introduced a new option "fts" (file time stamp) for "--with-native-compile", it will let the function "comp-el-to-eln-rel-filename" to get the .eln name by reading the .el/.el.gz file timestamp, make emacs faster on Windows. On my Windows testing env, the  Emacs startup up time was reduced from 8s to 6.5s with 386 packages, speedup ~20%.

The option can be combined with the exists option "aot", like "--with-native-compile=aot,fts".

This patch is safe for Emacs Windows release, which building with native compile enabled, but does not ship the libgccjit.dll. So the shipped *.eln is like constant files, user won't be able to recompile one; then this patch is safe for the scenario. 

Please help review this new option. Thanks

Best Regards
Lin

[0001-New-option-for-comp-.eln-file-name-by-the-file-times.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lin Sun <sunlin7 <at> hotmail.com>, Andrea Corallo <acorallo <at> gnu.org>
Cc: 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the file
 timestamp of *.el
Date: Sat, 10 May 2025 10:03:43 +0300

> From: Lin Sun <sunlin7 <at> hotmail.com>
> Date: Sat, 10 May 2025 00:07:52 +0000
> 
> Currently Emacs with native comp will comp .eln file name by the full context of the .el or .el.gz file in function "comp-el-to-eln-rel-filename".
> For example, emacs startup will require the simple.elc, then have to read full of simple.el.gz to calculate the name simple-X-Y.eln. 
> 
> That's slow on Windows.
> 
> This patch introduced a new option "fts" (file time stamp) for "--with-native-compile", it will let the function "comp-el-to-eln-rel-filename" to get the .eln name by reading the .el/.el.gz file timestamp, make emacs faster on Windows. On my Windows testing env, the  Emacs startup up time was reduced from 8s to 6.5s with 386 packages, speedup ~20%.

Is this with a cold disk cache or a warm disk cache?  IOW, if you
start Emacs, then kill the Emacs session, and then start it again, do
you still measure 8 sec or do you measure a significantly shorter
time?

Also, is your anti-virus software configured to ignore *.eln files in
the directories where you install them?  The *.eln files are DLLs as
far as Windows is concerned, so it's executable code, and anti-virus
software will therefore generally scan them when accessed or loaded,
which could significantly slow down loading.

Anyway, unless Andrea (CC'ed) enthusiastically embraces this, I don't
think it's correct to install this feature.  Time stamps are too
unreliable for this job:

  . it is easy to "fake" file's time using 'touch' 'cp -p' and similar
    techniques; in fact, the Emacs "make install" uses that
  . on a modern system, it is quite possible to have a newer file have
    the same time stamp as an older file, due to relatively low
    resolution of file time stamps
  . Emacs on MS-Windows has a 1-sec granularity of file time stamps,
    which makes the above even worse

All of the above means that using the file time stamps risks loading
incorrect *.eln files, which could crash Emacs.  So up front, I don't
think we should have this feature, on Windows or elsewhere.

Thanks.

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

From: Lin Sun <sunlin7 <at> hotmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Andrea Corallo <acorallo <at> gnu.org>
Cc: "78340 <at> debbugs.gnu.org" <78340 <at> debbugs.gnu.org>
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Sat, 10 May 2025 15:28:15 +0000

From: Eli Zaretskii <eliz <at> gnu.org>
Sent: Saturday, May 10, 2025 07:03 AM
> Is this with a cold disk cache or a warm disk cache?  IOW, if you
> start Emacs, then kill the Emacs session, and then start it again, do
> you still measure 8 sec or do you measure a significantly shorter time?

It's an average time, I run it on same environment ~5 times then cal the value. 

> Also, is your anti-virus software configured to ignore *.eln files in
> the directories where you install them?  The *.eln files are DLLs as
> far as Windows is concerned, so it's executable code, and anti-virus
> software will therefore generally scan them when accessed or loaded,
> which could significantly slow down loading.

I tested on same environment, with this patch it will speedup Emacs loading files.

> Anyway, unless Andrea (CC'ed) enthusiastically embraces this, I don't
> think it's correct to install this feature.  Time stamps are too
> unreliable for this job:
> 
>  . it is easy to "fake" file's time using 'touch' 'cp -p' and similar
>    techniques; in fact, the Emacs "make install" uses that
>  . on a modern system, it is quite possible to have a newer file have
>    the same time stamp as an older file, due to relatively low
>    resolution of file time stamps
>  . Emacs on MS-Windows has a 1-sec granularity of file time stamps,
>    which makes the above even worse
>
> All of the above means that using the file time stamps risks loading
> incorrect *.eln files, which could crash Emacs.  So up front, I don't
> think we should have this feature, on Windows or elsewhere.

The .eln file is named in filename-X-Y.eln format, timestamp change maybe lead that Emacs can not find the .eln file, then Emacs will use the .elc/.el file, still works.

All these won't compare to a malicious user compiling a .so/.dll file and replacing an exists .eln, reading full content to calculate the file name won't stop that risk.

This option gives Emacs user a chance to build a faster Emacs on their local, especially for Emacs on Windows. And it's not a default option, won't bother the user who don't build the Emacs themself.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lin Sun <sunlin7 <at> hotmail.com>
Cc: acorallo <at> gnu.org, 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Sat, 10 May 2025 18:46:15 +0300

> From: Lin Sun <sunlin7 <at> hotmail.com>
> CC: "78340 <at> debbugs.gnu.org" <78340 <at> debbugs.gnu.org>
> Date: Sat, 10 May 2025 15:28:15 +0000
> 
> > Also, is your anti-virus software configured to ignore *.eln files in
> > the directories where you install them?  The *.eln files are DLLs as
> > far as Windows is concerned, so it's executable code, and anti-virus
> > software will therefore generally scan them when accessed or loaded,
> > which could significantly slow down loading.
> 
> I tested on same environment, with this patch it will speedup Emacs loading files.

I asked specifically about anti-virus: did you try to configure it so
as not to scan the *.eln files?

> >  . it is easy to "fake" file's time using 'touch' 'cp -p' and similar
> >    techniques; in fact, the Emacs "make install" uses that
> >  . on a modern system, it is quite possible to have a newer file have
> >    the same time stamp as an older file, due to relatively low
> >    resolution of file time stamps
> >  . Emacs on MS-Windows has a 1-sec granularity of file time stamps,
> >    which makes the above even worse
> >
> > All of the above means that using the file time stamps risks loading
> > incorrect *.eln files, which could crash Emacs.  So up front, I don't
> > think we should have this feature, on Windows or elsewhere.
> 
> The .eln file is named in filename-X-Y.eln format, timestamp change maybe lead that Emacs can not find the .eln file, then Emacs will use the .elc/.el file, still works.

I'm actually bothered by the false-positive case: when the time stamp
matches, but the file is wrong.  These 2 hashes are there for a very
good reason: loading an incompatible file could cause fatal failures
and loss of data and edits.

> This option gives Emacs user a chance to build a faster Emacs on their local, especially for Emacs on Windows. And it's not a default option, won't bother the user who don't build the Emacs themself.

Sorry, I don't think it's right to add options that could risk
crashing Emacs.  What kind of project are we if we add such options,
even if they are off by default?

So let's wait for Andrea to provide his opinion on this.

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

From: Andrea Corallo <acorallo <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Lin Sun <sunlin7 <at> hotmail.com>, 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Mon, 12 May 2025 05:30:32 -0400

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Lin Sun <sunlin7 <at> hotmail.com>
>> Date: Sat, 10 May 2025 00:07:52 +0000
>> 
>> Currently Emacs with native comp will comp .eln file name by the
>> full context of the .el or .el.gz file in function
>> "comp-el-to-eln-rel-filename".
>> For example, emacs startup will require the simple.elc, then have to read full of simple.el.gz to calculate the name simple-X-Y.eln. 
>> 
>> That's slow on Windows.
>> 
>> This patch introduced a new option "fts" (file time stamp) for
>> "--with-native-compile", it will let the function
>> "comp-el-to-eln-rel-filename" to get the .eln name by reading the
>> .el/.el.gz file timestamp, make emacs faster on Windows. On my
>> Windows testing env, the Emacs startup up time was reduced from 8s
>> to 6.5s with 386 packages, speedup ~20%.
>
> Is this with a cold disk cache or a warm disk cache?  IOW, if you
> start Emacs, then kill the Emacs session, and then start it again, do
> you still measure 8 sec or do you measure a significantly shorter
> time?
>
> Also, is your anti-virus software configured to ignore *.eln files in
> the directories where you install them?  The *.eln files are DLLs as
> far as Windows is concerned, so it's executable code, and anti-virus
> software will therefore generally scan them when accessed or loaded,
> which could significantly slow down loading.
>
> Anyway, unless Andrea (CC'ed) enthusiastically embraces this,

I don't "enthusiastically embrace" this :).

I share all the concerns you have listed, the hash system was
constructed exactly to rely on something more robust than timestamps.

I'm no Windows expert, why do why see such a performance hit on this
system?

  Andrea

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andrea Corallo <acorallo <at> gnu.org>
Cc: sunlin7 <at> hotmail.com, 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Mon, 12 May 2025 14:56:12 +0300

> From: Andrea Corallo <acorallo <at> gnu.org>
> Cc: Lin Sun <sunlin7 <at> hotmail.com>,  78340 <at> debbugs.gnu.org
> Date: Mon, 12 May 2025 05:30:32 -0400
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > Anyway, unless Andrea (CC'ed) enthusiastically embraces this,
> 
> I don't "enthusiastically embrace" this :).
> 
> I share all the concerns you have listed, the hash system was
> constructed exactly to rely on something more robust than timestamps.

Thanks, I presumed you'd think that.

> I'm no Windows expert, why do why see such a performance hit on this
> system?

Disk I/O is slower, so loading hundreds of packages at startup (which
is what the OP does, AFAIU) takes time.

On my MS-Windows system, the following command

  time emacs -Q --eval "(progn (message \"foo\") (kill-emacs))"

reports the following times:

  real    00h00m00.149s
  user    00h00m00.109s
  sys     00h00m00.031s

This loads the *.eln files for all the preloaded Lisp packages.  In my
case, there are 162 *.eln files loaded by the above command.  I don't
know how many *.eln files does the OP have that are loaded at startup,
but even 1500 of them should load in, like, 1.5 sec, which should be
still okay for startup (that is supposed to be a rare event).

So I personally don't see this as a serious problem, but maybe I'm
missing something.

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

From: Lynn Winebarger <owinebar <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sunlin7 <at> hotmail.com, Andrea Corallo <acorallo <at> gnu.org>,
 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Mon, 12 May 2025 08:02:56 -0400

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

If he has that many packages, his load path has hundreds of entries, and
any preloaded library will look in every one before finding it in the last
entry ...

On Mon, May 12, 2025, 7:57 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Andrea Corallo <acorallo <at> gnu.org>
> > Cc: Lin Sun <sunlin7 <at> hotmail.com>,  78340 <at> debbugs.gnu.org
> > Date: Mon, 12 May 2025 05:30:32 -0400
> >
> > Eli Zaretskii <eliz <at> gnu.org> writes:
> >
> > > Anyway, unless Andrea (CC'ed) enthusiastically embraces this,
> >
> > I don't "enthusiastically embrace" this :).
> >
> > I share all the concerns you have listed, the hash system was
> > constructed exactly to rely on something more robust than timestamps.
>
> Thanks, I presumed you'd think that.
>
> > I'm no Windows expert, why do why see such a performance hit on this
> > system?
>
> Disk I/O is slower, so loading hundreds of packages at startup (which
> is what the OP does, AFAIU) takes time.
>
> On my MS-Windows system, the following command
>
>   time emacs -Q --eval "(progn (message \"foo\") (kill-emacs))"
>
> reports the following times:
>
>   real    00h00m00.149s
>   user    00h00m00.109s
>   sys     00h00m00.031s
>
> This loads the *.eln files for all the preloaded Lisp packages.  In my
> case, there are 162 *.eln files loaded by the above command.  I don't
> know how many *.eln files does the OP have that are loaded at startup,
> but even 1500 of them should load in, like, 1.5 sec, which should be
> still okay for startup (that is supposed to be a rare event).
>
> So I personally don't see this as a serious problem, but maybe I'm
> missing something.
>
>
>
>

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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lynn Winebarger <owinebar <at> gmail.com>
Cc: sunlin7 <at> hotmail.com, acorallo <at> gnu.org, 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Mon, 12 May 2025 15:15:29 +0300

> From: Lynn Winebarger <owinebar <at> gmail.com>
> Date: Mon, 12 May 2025 08:02:56 -0400
> Cc: Andrea Corallo <acorallo <at> gnu.org>, sunlin7 <at> hotmail.com, 78340 <at> debbugs.gnu.org
> 
> If he has that many packages, his load path has hundreds of entries, and any preloaded library will look in
> every one before finding it in the last entry ...

Maybe so, but that's a separate problem.  The patch submitted in this
bug was to shortcut the hashing of the source *.el files, not to avoid
searching the extra directories.

If the number of directories is the issue, then my suggestion is not
to compile packages AOT, which AFAIU will cause all the *.eln files to
be written to a single subdirectory of the eln-cache (after
compiling them the first time each one is loaded).  But that is a
separate issue which IMO should be discussed with Stefan and Philip,
because it's related to how third-party packages are installed and
compiled.

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

From: Lynn Winebarger <owinebar <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sunlin7 <at> hotmail.com, acorallo <at> gnu.org, 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Mon, 12 May 2025 09:07:06 -0400

On Mon, May 12, 2025 at 8:16 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > From: Lynn Winebarger <owinebar <at> gmail.com>
> > Date: Mon, 12 May 2025 08:02:56 -0400
> > Cc: Andrea Corallo <acorallo <at> gnu.org>, sunlin7 <at> hotmail.com, 78340 <at> debbugs.gnu.org
> >
> > If he has that many packages, his load path has hundreds of entries, and any preloaded library will look in
> > every one before finding it in the last entry ...
>
> Maybe so, but that's a separate problem.  The patch submitted in this
> bug was to shortcut the hashing of the source *.el files, not to avoid
> searching the extra directories.
>
> If the number of directories is the issue, then my suggestion is not
> to compile packages AOT, which AFAIU will cause all the *.eln files to
> be written to a single subdirectory of the eln-cache (after
> compiling them the first time each one is loaded).  But that is a
> separate issue which IMO should be discussed with Stefan and Philip,
> because it's related to how third-party packages are installed and
> compiled.

It's not the AOT that's the issue, it's the search for the source file
to compute the content hash.  But otherwise, you're right, it's a
general issue with package management.

Lynn

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

From: Lynn Winebarger <owinebar <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sunlin7 <at> hotmail.com, acorallo <at> gnu.org, 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Mon, 12 May 2025 09:18:02 -0400

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

On Mon, May 12, 2025, 9:07 AM Lynn Winebarger <owinebar <at> gmail.com> wrote:

> On Mon, May 12, 2025 at 8:16 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> >
> > > From: Lynn Winebarger <owinebar <at> gmail.com>
> > > Date: Mon, 12 May 2025 08:02:56 -0400
> > > Cc: Andrea Corallo <acorallo <at> gnu.org>, sunlin7 <at> hotmail.com,
> 78340 <at> debbugs.gnu.org
> > >
> > > If he has that many packages, his load path has hundreds of entries,
> and any preloaded library will look in
> > > every one before finding it in the last entry ...
> >
> > Maybe so, but that's a separate problem.  The patch submitted in this
> > bug was to shortcut the hashing of the source *.el files, not to avoid
> > searching the extra directories.
> >
> > If the number of directories is the issue, then my suggestion is not
> > to compile packages AOT, which AFAIU will cause all the *.eln files to
> > be written to a single subdirectory of the eln-cache (after
> > compiling them the first time each one is loaded).  But that is a
> > separate issue which IMO should be discussed with Stefan and Philip,
> > because it's related to how third-party packages are installed and
> > compiled.
>
> It's not the AOT that's the issue, it's the search for the source file
> to compute the content hash.  But otherwise, you're right, it's a
> general issue with package management.
>

That being said, a profile of the startup (without the patch) would be the
best way to tell where the issue is.
Probably should have led with that.

Lynn

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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lynn Winebarger <owinebar <at> gmail.com>
Cc: sunlin7 <at> hotmail.com, acorallo <at> gnu.org, 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Mon, 12 May 2025 16:32:56 +0300

> From: Lynn Winebarger <owinebar <at> gmail.com>
> Date: Mon, 12 May 2025 09:07:06 -0400
> Cc: acorallo <at> gnu.org, sunlin7 <at> hotmail.com, 78340 <at> debbugs.gnu.org
> 
> On Mon, May 12, 2025 at 8:16 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> >
> > > From: Lynn Winebarger <owinebar <at> gmail.com>
> > > Date: Mon, 12 May 2025 08:02:56 -0400
> > > Cc: Andrea Corallo <acorallo <at> gnu.org>, sunlin7 <at> hotmail.com, 78340 <at> debbugs.gnu.org
> > >
> > > If he has that many packages, his load path has hundreds of entries, and any preloaded library will look in
> > > every one before finding it in the last entry ...
> >
> > Maybe so, but that's a separate problem.  The patch submitted in this
> > bug was to shortcut the hashing of the source *.el files, not to avoid
> > searching the extra directories.
> >
> > If the number of directories is the issue, then my suggestion is not
> > to compile packages AOT, which AFAIU will cause all the *.eln files to
> > be written to a single subdirectory of the eln-cache (after
> > compiling them the first time each one is loaded).  But that is a
> > separate issue which IMO should be discussed with Stefan and Philip,
> > because it's related to how third-party packages are installed and
> > compiled.
> 
> It's not the AOT that's the issue, it's the search for the source file
> to compute the content hash.

Then the time I measured includes that, for the preloaded packages.

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

From: Lynn Winebarger <owinebar <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sunlin7 <at> hotmail.com, acorallo <at> gnu.org, 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Mon, 12 May 2025 09:42:06 -0400

On Mon, May 12, 2025 at 9:33 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > From: Lynn Winebarger <owinebar <at> gmail.com>
> > Date: Mon, 12 May 2025 09:07:06 -0400
> > Cc: acorallo <at> gnu.org, sunlin7 <at> hotmail.com, 78340 <at> debbugs.gnu.org
> >
> > On Mon, May 12, 2025 at 8:16 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> > >
> > > > From: Lynn Winebarger <owinebar <at> gmail.com>
> > > > Date: Mon, 12 May 2025 08:02:56 -0400
> > > > Cc: Andrea Corallo <acorallo <at> gnu.org>, sunlin7 <at> hotmail.com, 78340 <at> debbugs.gnu.org
> > > >
> > > > If he has that many packages, his load path has hundreds of entries, and any preloaded library will look in
> > > > every one before finding it in the last entry ...
> > >
> > > Maybe so, but that's a separate problem.  The patch submitted in this
> > > bug was to shortcut the hashing of the source *.el files, not to avoid
> > > searching the extra directories.
> > >
> > > If the number of directories is the issue, then my suggestion is not
> > > to compile packages AOT, which AFAIU will cause all the *.eln files to
> > > be written to a single subdirectory of the eln-cache (after
> > > compiling them the first time each one is loaded).  But that is a
> > > separate issue which IMO should be discussed with Stefan and Philip,
> > > because it's related to how third-party packages are installed and
> > > compiled.
> >
> > It's not the AOT that's the issue, it's the search for the source file
> > to compute the content hash.
>
> Then the time I measured includes that, for the preloaded packages.

That's true - he's only seeing a 20% speedup.

I don't really follow the logic on why the content hash of the current
source is required to keep emacs from *crashing*, as opposed to just
being inconsistent with the current source.  That state of affairs has
always been considered acceptable for byte compiled libraries.
So, instead of checking file times, why not just put the content hash
in the .elc, and if loading the elc would be acceptable (determined by
file times), then use the value from the elc file (stored in the
comment block at the start so the Fread procedure is not required).

Lynn

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

From: Lynn Winebarger <owinebar <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sunlin7 <at> hotmail.com, acorallo <at> gnu.org, 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Mon, 12 May 2025 09:49:19 -0400

On Mon, May 12, 2025 at 9:42 AM Lynn Winebarger <owinebar <at> gmail.com> wrote:
>
> On Mon, May 12, 2025 at 9:33 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> >
> > > From: Lynn Winebarger <owinebar <at> gmail.com>
> > > Date: Mon, 12 May 2025 09:07:06 -0400
> > > Cc: acorallo <at> gnu.org, sunlin7 <at> hotmail.com, 78340 <at> debbugs.gnu.org
> > >
> > > On Mon, May 12, 2025 at 8:16 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> > > >
> > > > > From: Lynn Winebarger <owinebar <at> gmail.com>
> > > > > Date: Mon, 12 May 2025 08:02:56 -0400
> > > > > Cc: Andrea Corallo <acorallo <at> gnu.org>, sunlin7 <at> hotmail.com, 78340 <at> debbugs.gnu.org
> > > > >
> > > > > If he has that many packages, his load path has hundreds of entries, and any preloaded library will look in
> > > > > every one before finding it in the last entry ...
> > > >
> > > > Maybe so, but that's a separate problem.  The patch submitted in this
> > > > bug was to shortcut the hashing of the source *.el files, not to avoid
> > > > searching the extra directories.
> > > >
> > > > If the number of directories is the issue, then my suggestion is not
> > > > to compile packages AOT, which AFAIU will cause all the *.eln files to
> > > > be written to a single subdirectory of the eln-cache (after
> > > > compiling them the first time each one is loaded).  But that is a
> > > > separate issue which IMO should be discussed with Stefan and Philip,
> > > > because it's related to how third-party packages are installed and
> > > > compiled.
> > >
> > > It's not the AOT that's the issue, it's the search for the source file
> > > to compute the content hash.
> >
> > Then the time I measured includes that, for the preloaded packages.
>
> That's true - he's only seeing a 20% speedup.
>
> I don't really follow the logic on why the content hash of the current
> source is required to keep emacs from *crashing*, as opposed to just
> being inconsistent with the current source.  That state of affairs has
> always been considered acceptable for byte compiled libraries.
> So, instead of checking file times, why not just put the content hash
> in the .elc, and if loading the elc would be acceptable (determined by
> file times), then use the value from the elc file (stored in the
> comment block at the start so the Fread procedure is not required).

Or, for system-installed libraries, treat them like doc strings and
store them in a file in the etc subdirectory?
Those are not mutually exclusive.  The first would apply to all .elns,
while the second would skip the whole locate-file procedure for system
libraries (if the eln is in the system directory as well).

Lynn

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lynn Winebarger <owinebar <at> gmail.com>
Cc: sunlin7 <at> hotmail.com, acorallo <at> gnu.org, 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Mon, 12 May 2025 16:53:01 +0300

> From: Lynn Winebarger <owinebar <at> gmail.com>
> Date: Mon, 12 May 2025 09:42:06 -0400
> Cc: acorallo <at> gnu.org, sunlin7 <at> hotmail.com, 78340 <at> debbugs.gnu.org
> 
> I don't really follow the logic on why the content hash of the current
> source is required to keep emacs from *crashing*, as opposed to just
> being inconsistent with the current source.

How else can you make sure the native code is safe to run?  E.g., what
if the code calls primitives that don't exist in Emacs, or their
signature is different?

> That state of affairs has always been considered acceptable for byte
> compiled libraries.

It was a requirement to prevent crashing a session due to loading
incompatible native code, because loading byte-code doesn't crash.

> So, instead of checking file times, why not just put the content hash
> in the .elc, and if loading the elc would be acceptable (determined by
> file times), then use the value from the elc file (stored in the
> comment block at the start so the Fread procedure is not required).

I don't see how the time stamp of the .elc file is more reliable than
of the .el file.

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

From: Lynn Winebarger <owinebar <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sunlin7 <at> hotmail.com, acorallo <at> gnu.org, 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Mon, 12 May 2025 10:05:36 -0400

On Mon, May 12, 2025 at 9:53 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > From: Lynn Winebarger <owinebar <at> gmail.com>
> > Date: Mon, 12 May 2025 09:42:06 -0400
> > Cc: acorallo <at> gnu.org, sunlin7 <at> hotmail.com, 78340 <at> debbugs.gnu.org
> >
> > I don't really follow the logic on why the content hash of the current
> > source is required to keep emacs from *crashing*, as opposed to just
> > being inconsistent with the current source.
>
> How else can you make sure the native code is safe to run?  E.g., what
> if the code calls primitives that don't exist in Emacs, or their
> signature is different?

That's what the ABI hash does.  The content hash doesn't prove
anything about that.

>
> > That state of affairs has always been considered acceptable for byte
> > compiled libraries.
>
> It was a requirement to prevent crashing a session due to loading
> incompatible native code, because loading byte-code doesn't crash.
>
> > So, instead of checking file times, why not just put the content hash
> > in the .elc, and if loading the elc would be acceptable (determined by
> > file times), then use the value from the elc file (stored in the
> > comment block at the start so the Fread procedure is not required).
>
> I don't see how the time stamp of the .elc file is more reliable than
> of the .el file.

If the byte-compiler notes in its output (by this comment tag) "this
elc was compiled from a source file with content hash X", then why
would X not be satisfactory for determining if the .eln was generated
from valid elisp code?  If someone can write to the elc file, they can
probably write to the .eln file as well and change the embedded
content hash to whatever they want if that's the concern.

Lynn

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lynn Winebarger <owinebar <at> gmail.com>
Cc: sunlin7 <at> hotmail.com, acorallo <at> gnu.org, 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Mon, 12 May 2025 17:23:52 +0300

> From: Lynn Winebarger <owinebar <at> gmail.com>
> Date: Mon, 12 May 2025 10:05:36 -0400
> Cc: acorallo <at> gnu.org, sunlin7 <at> hotmail.com, 78340 <at> debbugs.gnu.org
> 
> On Mon, May 12, 2025 at 9:53 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> >
> > > From: Lynn Winebarger <owinebar <at> gmail.com>
> > > Date: Mon, 12 May 2025 09:42:06 -0400
> > > Cc: acorallo <at> gnu.org, sunlin7 <at> hotmail.com, 78340 <at> debbugs.gnu.org
> > >
> > > I don't really follow the logic on why the content hash of the current
> > > source is required to keep emacs from *crashing*, as opposed to just
> > > being inconsistent with the current source.
> >
> > How else can you make sure the native code is safe to run?  E.g., what
> > if the code calls primitives that don't exist in Emacs, or their
> > signature is different?
> 
> That's what the ABI hash does.  The content hash doesn't prove
> anything about that.

No, the ABI hash is the evidence for the Emacs binary.  We need
evidence for the .eln file's code.

> > > So, instead of checking file times, why not just put the content hash
> > > in the .elc, and if loading the elc would be acceptable (determined by
> > > file times), then use the value from the elc file (stored in the
> > > comment block at the start so the Fread procedure is not required).
> >
> > I don't see how the time stamp of the .elc file is more reliable than
> > of the .el file.
> 
> If the byte-compiler notes in its output (by this comment tag) "this
> elc was compiled from a source file with content hash X", then why
> would X not be satisfactory for determining if the .eln was generated
> from valid elisp code?  If someone can write to the elc file, they can
> probably write to the .eln file as well and change the embedded
> content hash to whatever they want if that's the concern.

You've changed the subject: I was talking about the time stamp.

As for your proposal, I'm not sure it is better, let alone
significantly better.  Emacs currently looks for the .eln without
reading the .elc file, only because the .elc file was found.  You
suggest reading the .elc file, which will probably be slow on Windows
for the same reason computing the content hash is slow.  (We currently
don't open the .elc file on Windows because of this slowness, we just
ask the filesystem whether the file exists.)

So it is not clear to me that this complication (which will as a side
effect make the .elc files incompatible with previous Emacs versions)
is justified.  Someone will have to show a significant speedup to make
it worth considering.

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

From: Lynn Winebarger <owinebar <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sunlin7 <at> hotmail.com, acorallo <at> gnu.org, 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Mon, 12 May 2025 11:37:13 -0400

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

On Mon, May 12, 2025, 10:24 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Lynn Winebarger <owinebar <at> gmail.com>
> > Date: Mon, 12 May 2025 10:05:36 -0400
> > Cc: acorallo <at> gnu.org, sunlin7 <at> hotmail.com, 78340 <at> debbugs.gnu.org
> >
> > On Mon, May 12, 2025 at 9:53 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> > >
> > > > From: Lynn Winebarger <owinebar <at> gmail.com>
> > > > Date: Mon, 12 May 2025 09:42:06 -0400
> > > > Cc: acorallo <at> gnu.org, sunlin7 <at> hotmail.com, 78340 <at> debbugs.gnu.org
> > > >
> > > > I don't really follow the logic on why the content hash of the
> current
> > > > source is required to keep emacs from *crashing*, as opposed to just
> > > > being inconsistent with the current source.
> > >
> > > How else can you make sure the native code is safe to run?  E.g., what
> > > if the code calls primitives that don't exist in Emacs, or their
> > > signature is different?
> >
> > That's what the ABI hash does.  The content hash doesn't prove
> > anything about that.
>
> No, the ABI hash is the evidence for the Emacs binary.  We need
> evidence for the .eln file's code


So the idea is that the content hash is supposed to certify that the ELN
was derived from some valid lisp code, and the existence of that source in
the path is the trusted authority for valid lisp code?
Looking in src/comp.c,
------------------
  /* We create eln filenames with an hash in order to look-up these
     starting from the source filename, IOW have a relation

     /absolute/path/filename.el + content ->
     eln-cache/filename-path_hash-content_hash.eln.

     'dlopen' can return the same handle if two shared with the same
     filename are loaded in two different times (even if the first was
     deleted!).  To prevent this scenario the source file content is
     included in the hashing algorithm.

     As at any point in time no more then one file can exist with the
     same filename, should be possible to clean up all
     filename-path_hash-* except the most recent one (or the new one
     being recompiled).

     As installing .eln files compiled during the build changes their
     absolute path we need an hashing mechanism that is not sensitive
     to that.  For this we replace if match PATH_DUMPLOADSEARCH or
     *PATH_REL_LOADSEARCH with '//' before computing the hash.  */
------------------------

There's nothing there saying the check is to make sure that the code in the
.eln is safe, only that the eln filename is unique when a library is
recompiled with different source.  There's nothing that implies that it
would be *unsafe* to load a library generated from a version with a
different content hash than the one that happens to exist on disk at the
moment.  A compile-time time stamp would probably be adequate for the
stated purpose of ensuring a unqiue file name.
I don't see anything in comp.c storing the content hash in the .eln file
for verification purposes.  It looks like if I modify an elisp library,
then copy the existing .eln to have a filename with the content hash from
the new version, but without any modification of the eln contents, it
should load the same way the older byte-compiled version would.  Are you
(or Andrea) saying that would risk a crash?
Am I missing something?


> > > > So, instead of checking file times, why not just put the content hash
> > > > in the .elc, and if loading the elc would be acceptable (determined
> by
> > > > file times), then use the value from the elc file (stored in the
> > > > comment block at the start so the Fread procedure is not required).
> > >
> > > I don't see how the time stamp of the .elc file is more reliable than
> > > of the .el file.
> >
> > If the byte-compiler notes in its output (by this comment tag) "this
> > elc was compiled from a source file with content hash X", then why
> > would X not be satisfactory for determining if the .eln was generated
> > from valid elisp code?  If someone can write to the elc file, they can
> > probably write to the .eln file as well and change the embedded
> > content hash to whatever they want if that's the concern.
>
> You've changed the subject: I was talking about the time stamp.
>
> As for your proposal, I'm not sure it is better, let alone
> significantly better.  Emacs currently looks for the .eln without
> reading the .elc file, only because the .elc file was found.  You
> suggest reading the .elc file, which will probably be slow on Windows
> for the same reason computing the content hash is slow.  (We currently
> don't open the .elc file on Windows because of this slowness, we just
> ask the filesystem whether the file exists.)
>
> So it is not clear to me that this complication (which will as a side
> effect make the .elc files incompatible with previous Emacs versions)
> is justified.  Someone will have to show a significant speedup to make
> it worth considering.
>


I didn't suggest reading the whole file, only the leading comment block. If
the performance impact is from reading a single block of a file, the you're
right.  I thought the issue was having to do a computation linear in the
size of the file.

For your parenthetical, how would adding a line like
; content-hash: ...
after the existing comment block make the file incompatible with older
versions?  load just processes the file through "read" anyway, and that
wouldn't be affected by a comment line.  Unless I've
seriously misread src/lread.c.

If the only purpose of the content hash is to certify that some valid elisp
source (with respect to the running emacs) is known to exist, then emacs
should be able to keep a table in memory loaded at startup and just consult
that for verification.  That should address any concern about crashing, at
least, unless I have completely misunderstood the concern about (which is
very possible).

Lynn

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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lynn Winebarger <owinebar <at> gmail.com>
Cc: sunlin7 <at> hotmail.com, acorallo <at> gnu.org, 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Mon, 12 May 2025 19:13:55 +0300

> From: Lynn Winebarger <owinebar <at> gmail.com>
> Date: Mon, 12 May 2025 11:37:13 -0400
> Cc: acorallo <at> gnu.org, sunlin7 <at> hotmail.com, 78340 <at> debbugs.gnu.org
> 
> There's nothing there saying the check is to make sure that the code in the .eln is safe, only that the eln
> filename is unique when a library is recompiled with different source.  There's nothing that implies that it
> would be *unsafe* to load a library generated from a version with a different content hash than the one that
> happens to exist on disk at the moment.  A compile-time time stamp would probably be adequate for the
> stated purpose of ensuring a unqiue file name.

So now you are saying that what we do is NOT enough, and more should
be done to make sure the loaded .eln file is safe?

Or are you saying that, because (in your opinion) the current tests
are not enough, we could well throw them away completely, because it
will save us some milliseconds per file?

Or what is it that you are saying?

Whatever you claim about the current code, it withstood the test of 2
major Emacs releases: I've yet top hear any report about Emacs
crashing because it loaded an incompatible .eln file.  That's not
nothing.

> I don't see anything in comp.c storing the content hash in the .eln file for verification purposes.  It looks like
> if I modify an elisp library, then copy the existing .eln to have a filename with the content hash from the new
> version, but without any modification of the eln contents, it should load the same way the older byte-compiled
> version would.  Are you (or Andrea) saying that would risk a crash?  
> Am I missing something?

Maliciously tampering with files can still crash Emacs, yes.  That's
not what those safeguards are about.

>  So it is not clear to me that this complication (which will as a side
>  effect make the .elc files incompatible with previous Emacs versions)
>  is justified.  Someone will have to show a significant speedup to make
>  it worth considering.
> 
> I didn't suggest reading the whole file, only the leading comment block. If the performance impact is from
> reading a single block of a file, the you're right.  I thought the issue was having to do a computation linear in
> the size of the file.

Not on Windows, at least that's not my guess.  The CPU processing
power is the same on Windows as on other systems, but I/O is slower.
Opening a file and reading some of it is slower.

Of course, this is just a guess, and someone needs to measure this.

> If the only purpose of the content hash is to certify that some valid elisp source (with respect to the running
> emacs) is known to exist, then emacs should be able to keep a table in memory loaded at startup and just
> consult that for verification.  That should address any concern about crashing, at least, unless I have
> completely misunderstood the concern about (which is very possible).

You lost me here.  What would be the contents of that table, how it
will be produced, and how it will be used?

And, btw, what does all that have to do with the current bug, which is
very specific and narrow?  It sounds like you'd like to start a
discussion on emacs-devel where you would like to suggest how to
redesign the way we validate and load *.eln files.  It's a separate
discussion with much broader scope and implications.

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

From: Lin Sun <sunlin7 <at> hotmail.com>
To: Lynn Winebarger <owinebar <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: "acorallo <at> gnu.org" <acorallo <at> gnu.org>,
 "78340 <at> debbugs.gnu.org" <78340 <at> debbugs.gnu.org>
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Mon, 12 May 2025 16:38:06 +0000

From: Eli Zaretskii <eliz <at> gnu.org>
Sent: Monday, May 12, 2025 02:24 PM
> ... which will probably be slow on Windows
> for the same reason computing the content hash is slow...

I want highlight reading a .el file will call windows api OpenFile/ReadFile.../CloseFile.
Here is an example that Emacs try to read the .el file to calculate the .eln name, it had called "ReadFile()" 6 time to read full content.
If Emacs uses timestamp, just call Getattribute() function, will reduce counts on windows file api calling.

"CreateFile","emacs-orig\lisp\emacs-lisp\byte-opt.el","SUCCESS","Desired Access: Generic Read, Disposition: Open
"ReadFile","emacs-orig\lisp\emacs-lisp\byte-opt.el","SUCCESS","Offset: 0, Length: 32,768, Priority: Normal"
"ReadFile","emacs-orig\lisp\emacs-lisp\byte-opt.el","SUCCESS","Offset: 32,768, Length: 32,768"
"ReadFile","emacs-orig\lisp\emacs-lisp\byte-opt.el","SUCCESS","Offset: 65,536, Length: 32,768"
"ReadFile","emacs-orig\lisp\emacs-lisp\byte-opt.el","SUCCESS","Offset: 98,304, Length: 32,768"
"ReadFile","emacs-orig\lisp\emacs-lisp\byte-opt.el","SUCCESS","Offset: 131,072, Length: 5,129"
"ReadFile","emacs-orig\lisp\emacs-lisp\byte-opt.el","END OF FILE","Offset: 136,201, Length: 24,576"
"CloseFile","emacs-orig\lisp\emacs-lisp\byte-opt.el","SUCCESS",""
"CreateFile",".emacs.d\eln-cache\31.0.50-e92228c5\byte-opt-9c5f25f5-4dacc99e.eln","SUCCESS"

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lin Sun <sunlin7 <at> hotmail.com>
Cc: acorallo <at> gnu.org, owinebar <at> gmail.com, 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Mon, 12 May 2025 22:11:02 +0300

> From: Lin Sun <sunlin7 <at> hotmail.com>
> CC: "acorallo <at> gnu.org" <acorallo <at> gnu.org>, "78340 <at> debbugs.gnu.org"
> 	<78340 <at> debbugs.gnu.org>
> Date: Mon, 12 May 2025 16:38:06 +0000
> msip_labels:
> 
> From: Eli Zaretskii <eliz <at> gnu.org>
> Sent: Monday, May 12, 2025 02:24 PM
> > ... which will probably be slow on Windows
> > for the same reason computing the content hash is slow...
> 
> I want highlight reading a .el file will call windows api OpenFile/ReadFile.../CloseFile.
> Here is an example that Emacs try to read the .el file to calculate the .eln name, it had called "ReadFile()" 6 time to read full content.

Yes, because Emacs reads files in chunks of 32KB.

> If Emacs uses timestamp, just call Getattribute() function, will reduce counts on windows file api calling.

I tried to explain why relying on time stamps is not reliable enough
in this case, it brings too high risk of loading an incompatible .eln
file.

In Emacs, we value correctness before speed.

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

From: Lynn Winebarger <owinebar <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sunlin7 <at> hotmail.com, acorallo <at> gnu.org, 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Mon, 12 May 2025 19:47:54 -0400

On Mon, May 12, 2025 at 12:14 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > From: Lynn Winebarger <owinebar <at> gmail.com>
> > Date: Mon, 12 May 2025 11:37:13 -0400
> > Cc: acorallo <at> gnu.org, sunlin7 <at> hotmail.com, 78340 <at> debbugs.gnu.org
> >
> > There's nothing there saying the check is to make sure that the code in the .eln is safe, only that the eln
> > filename is unique when a library is recompiled with different source.  There's nothing that implies that it
> > would be *unsafe* to load a library generated from a version with a different content hash than the one that
> > happens to exist on disk at the moment.  A compile-time time stamp would probably be adequate for the
> > stated purpose of ensuring a unqiue file name.
>
> So now you are saying that what we do is NOT enough, and more should
> be done to make sure the loaded .eln file is safe?
>
> Or are you saying that, because (in your opinion) the current tests
> are not enough, we could well throw them away completely, because it
> will save us some milliseconds per file?
>
> Or what is it that you are saying?

I claimed none of those things.  I was trying to understand what you
meant by "No, the ABI hash is the evidence for the Emacs binary.  We
need evidence for the .eln file's code."  That way I might be able to
assist the reporter in coming up with an approach that would solve his
issue and have a chance of being accepted.  I am sympathetic to
wanting to make emacs startup take less time in general.
I elaborated my guesses as to what the reason might be because I
hadn't seen a concrete explanation of the hazard being avoided.
Functionally, as far as I can tell, the content hash only serves to
ensure that ELN files generated from distinct source files will have
distinct names, so that dlopen will not confuse them, even if they map
to the same relative path from the root lisp directory.    That's
certainly how I read the comment in the source.  If I am missing
something, please correct me.

>
> Whatever you claim about the current code, it withstood the test of 2
> major Emacs releases: I've yet top hear any report about Emacs
> crashing because it loaded an incompatible .eln file.  That's not
> nothing.

I'm not sure why you seem to be taking offense.  It's clearly a
conservative approach that has avoided failures that might have given
the system a reputation for unreliability.

> > I don't see anything in comp.c storing the content hash in the .eln file for verification purposes.  It looks like
> > if I modify an elisp library, then copy the existing .eln to have a filename with the content hash from the new
> > version, but without any modification of the eln contents, it should load the same way the older byte-compiled
> > version would.  Are you (or Andrea) saying that would risk a crash?
> > Am I missing something?
>
> Maliciously tampering with files can still crash Emacs, yes.  That's
> not what those safeguards are about.
>
I don't think the test I suggested as an experiment to determine the
actual possibility of crashes even approaches "maliciously tampering
with files".  To the best of my ability to judge what would happen
based on the compiler infrastructure, there's no hazard in that
experiment of a crash happening, only that the new code would be
ignored, just as it would be if "load-prefer-newer" is nil for
byte-compiled files.  But I'm more than happy to be corrected by a
concrete example.  If I'm incorrect, I would prefer to know sooner
than later.

> > If the only purpose of the content hash is to certify that some valid elisp source (with respect to the running
> > emacs) is known to exist, then emacs should be able to keep a table in memory loaded at startup and just
> > consult that for verification.  That should address any concern about crashing, at least, unless I have
> > completely misunderstood the concern about (which is very possible).
>
> You lost me here.  What would be the contents of that table, how it
> will be produced, and how it will be used?

Actually, this very simple idea occurred to me in the course of this
discussion.  Just have the compilers (both byte- and native-) keep a
log file with the following items in each line:
--------
<source file path relative to load path (i.e. argument to "load" or require>
<file path relative to emacs lisp source directory as used by native compiler>
<file basename>-<md5 hash of relative path>
<content hash of source code that was compiled>
<basename of compiled library (whether .elc or .eln>
<content hash of compiled file>
--------
Emacs can load this log (part from the installation or invocation
location and part from the user emacs directory) at startup.  Then, at
least for compiled libraries when "load-prefer-newer" is nil, the
correct library to load can be determined entirely from internal
records, without hitting the disk at all, as long as the compiled file
exists where expected (and it can even be verified if required).  In
the worst case, the current behavior can still be followed.
With that approach, emacs actually has an authoritative source of
content hashes that its compiler has verified as proper elisp code (to
the extent that is the concern), and in the extremis, the library with
that name can be verified to be the output of the compiler (whether
from the byte compiler or the native compiler).  I don't think that's
necessary, but it would become possible, and it's not that hard to
obtain the option.  This would actually avoid a lot of performance
related issues with packages and long load-paths to boot that derive
from system libraries being the most likely to be required but at the
end of the path to be searched, although that is just a fringe
benefit.

>
> And, btw, what does all that have to do with the current bug, which is
> very specific and narrow?  It sounds like you'd like to start a
> discussion on emacs-devel where you would like to suggest how to
> redesign the way we validate and load *.eln files.  It's a separate
> discussion with much broader scope and implications.

Well, if the bug is specifically "add an option to use file stat times
to determine validity", then you're correct.  I took the reporter to
be saying "20% of my emacs startup time is due to this check, I'd like
to avoid that because I don't understand what the purpose of this
check is when this other check seems just as good, here's a patch".  I
think the approach I suggested above would be much more fruitful for
the OP if they want to contribute something with a chance of being
accepted.  It's not trivial like the approach implemented by the
submitted patch, but it isn't insurmountably complex or conceptually
difficult to grasp.

OTOH, I got engrossed in the issue at hand (as I understand it) and
totally forgot this was from bug-emacs and not emacs-devel already, so
you might be right about that being the appropriate forum for my last
suggestion above.

Lynn

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

From: Lynn Winebarger <owinebar <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sunlin7 <at> hotmail.com, acorallo <at> gnu.org, 78340 <at> debbugs.gnu.org
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the
 file timestamp of *.el
Date: Mon, 12 May 2025 19:52:00 -0400

On Mon, May 12, 2025 at 7:47 PM Lynn Winebarger <owinebar <at> gmail.com> wrote:
>
> On Mon, May 12, 2025 at 12:14 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
> >
> > > From: Lynn Winebarger <owinebar <at> gmail.com>
> > > Date: Mon, 12 May 2025 11:37:13 -0400
> > > Cc: acorallo <at> gnu.org, sunlin7 <at> hotmail.com, 78340 <at> debbugs.gnu.org
> > >
> > > There's nothing there saying the check is to make sure that the code in the .eln is safe, only that the eln
> > > filename is unique when a library is recompiled with different source.  There's nothing that implies that it
> > > would be *unsafe* to load a library generated from a version with a different content hash than the one that
> > > happens to exist on disk at the moment.  A compile-time time stamp would probably be adequate for the
> > > stated purpose of ensuring a unqiue file name.
> >
> > So now you are saying that what we do is NOT enough, and more should
> > be done to make sure the loaded .eln file is safe?
> >
> > Or are you saying that, because (in your opinion) the current tests
> > are not enough, we could well throw them away completely, because it
> > will save us some milliseconds per file?
> >
> > Or what is it that you are saying?
>
> I claimed none of those things.  I was trying to understand what you
> meant by "No, the ABI hash is the evidence for the Emacs binary.  We
> need evidence for the .eln file's code."  That way I might be able to
> assist the reporter in coming up with an approach that would solve his
> issue and have a chance of being accepted.  I am sympathetic to
> wanting to make emacs startup take less time in general.
> I elaborated my guesses as to what the reason might be because I
> hadn't seen a concrete explanation of the hazard being avoided.
> Functionally, as far as I can tell, the content hash only serves to
> ensure that ELN files generated from distinct source files will have
> distinct names, so that dlopen will not confuse them, even if they map
> to the same relative path from the root lisp directory.    That's
> certainly how I read the comment in the source.  If I am missing
> something, please correct me.
>
> >
> > Whatever you claim about the current code, it withstood the test of 2
> > major Emacs releases: I've yet top hear any report about Emacs
> > crashing because it loaded an incompatible .eln file.  That's not
> > nothing.
>
> I'm not sure why you seem to be taking offense.  It's clearly a
> conservative approach that has avoided failures that might have given
> the system a reputation for unreliability.
>
> > > I don't see anything in comp.c storing the content hash in the .eln file for verification purposes.  It looks like
> > > if I modify an elisp library, then copy the existing .eln to have a filename with the content hash from the new
> > > version, but without any modification of the eln contents, it should load the same way the older byte-compiled
> > > version would.  Are you (or Andrea) saying that would risk a crash?
> > > Am I missing something?
> >
> > Maliciously tampering with files can still crash Emacs, yes.  That's
> > not what those safeguards are about.
> >
> I don't think the test I suggested as an experiment to determine the
> actual possibility of crashes even approaches "maliciously tampering
> with files".  To the best of my ability to judge what would happen
> based on the compiler infrastructure, there's no hazard in that
> experiment of a crash happening, only that the new code would be
> ignored, just as it would be if "load-prefer-newer" is nil for
> byte-compiled files.  But I'm more than happy to be corrected by a
> concrete example.  If I'm incorrect, I would prefer to know sooner
> than later.
>
> > > If the only purpose of the content hash is to certify that some valid elisp source (with respect to the running
> > > emacs) is known to exist, then emacs should be able to keep a table in memory loaded at startup and just
> > > consult that for verification.  That should address any concern about crashing, at least, unless I have
> > > completely misunderstood the concern about (which is very possible).
> >
> > You lost me here.  What would be the contents of that table, how it
> > will be produced, and how it will be used?
>
> Actually, this very simple idea occurred to me in the course of this
> discussion.  Just have the compilers (both byte- and native-) keep a
> log file with the following items in each line:

I forgot the most important item!
> --------
> <source file path relative to load path (i.e. argument to "load" or require>
> <file path relative to emacs lisp source directory as used by native compiler>
> <file basename>-<md5 hash of relative path>
> <content hash of source code that was compiled>
<time stamp at which the compiled file was written> !!!
> <basename of compiled library (whether .elc or .eln>
> <content hash of compiled file>
> --------
> Emacs can load this log (part from the installation or invocation
> location and part from the user emacs directory) at startup.  Then, at
> least for compiled libraries when "load-prefer-newer" is nil, the
> correct library to load can be determined entirely from internal
> records, without hitting the disk at all, as long as the compiled file
> exists where expected (and it can even be verified if required).  In
> the worst case, the current behavior can still be followed.
> With that approach, emacs actually has an authoritative source of
> content hashes that its compiler has verified as proper elisp code (to
> the extent that is the concern), and in the extremis, the library with
> that name can be verified to be the output of the compiler (whether
> from the byte compiler or the native compiler).  I don't think that's
> necessary, but it would become possible, and it's not that hard to
> obtain the option.  This would actually avoid a lot of performance
> related issues with packages and long load-paths to boot that derive
> from system libraries being the most likely to be required but at the
> end of the path to be searched, although that is just a fringe
> benefit.
>
> >
> > And, btw, what does all that have to do with the current bug, which is
> > very specific and narrow?  It sounds like you'd like to start a
> > discussion on emacs-devel where you would like to suggest how to
> > redesign the way we validate and load *.eln files.  It's a separate
> > discussion with much broader scope and implications.
>
> Well, if the bug is specifically "add an option to use file stat times
> to determine validity", then you're correct.  I took the reporter to
> be saying "20% of my emacs startup time is due to this check, I'd like
> to avoid that because I don't understand what the purpose of this
> check is when this other check seems just as good, here's a patch".  I
> think the approach I suggested above would be much more fruitful for
> the OP if they want to contribute something with a chance of being
> accepted.  It's not trivial like the approach implemented by the
> submitted patch, but it isn't insurmountably complex or conceptually
> difficult to grasp.
>
> OTOH, I got engrossed in the issue at hand (as I understand it) and
> totally forgot this was from bug-emacs and not emacs-devel already, so
> you might be right about that being the appropriate forum for my last
> suggestion above.
>
> Lynn

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: sunlin7 <at> hotmail.com
Cc: 78340-done <at> debbugs.gnu.org, acorallo <at> gnu.org, owinebar <at> gmail.com
Subject: Re: bug#78340: [PATCH] New option for comp *.eln file name by the file
 timestamp of *.el
Date: Sat, 24 May 2025 12:01:59 +0300

> Cc: acorallo <at> gnu.org, owinebar <at> gmail.com, 78340 <at> debbugs.gnu.org
> Date: Mon, 12 May 2025 22:11:02 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> 
> > From: Lin Sun <sunlin7 <at> hotmail.com>
> > CC: "acorallo <at> gnu.org" <acorallo <at> gnu.org>, "78340 <at> debbugs.gnu.org"
> > 	<78340 <at> debbugs.gnu.org>
> > Date: Mon, 12 May 2025 16:38:06 +0000
> > msip_labels:
> > 
> > From: Eli Zaretskii <eliz <at> gnu.org>
> > Sent: Monday, May 12, 2025 02:24 PM
> > > ... which will probably be slow on Windows
> > > for the same reason computing the content hash is slow...
> > 
> > I want highlight reading a .el file will call windows api OpenFile/ReadFile.../CloseFile.
> > Here is an example that Emacs try to read the .el file to calculate the .eln name, it had called "ReadFile()" 6 time to read full content.
> 
> Yes, because Emacs reads files in chunks of 32KB.
> 
> > If Emacs uses timestamp, just call Getattribute() function, will reduce counts on windows file api calling.
> 
> I tried to explain why relying on time stamps is not reliable enough
> in this case, it brings too high risk of loading an incompatible .eln
> file.
> 
> In Emacs, we value correctness before speed.

No further comments, so I'm now closing this bug.

Previous Next

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