bug#4446: remove ###autoload from ediff-hook.el
Dan Nicolaescu
The ###autoload in ediff-hook.el is not needed, this file is in the
dumped image, so the autoload's only effect is to make loaddefs.el
bigger and increase the size of the dumped image.
The comment in the file says that the autoload is needed for XEmacs, but
that's not a good reason to keep it in the Emacs CVS, it's a one line
change to a file that almost never changes.
OK to remove the autoload line?
Leo
That is a big chunk of autoload.
Leo
Stefan Monnier
> dumped image, so the autoload's only effect is to make loaddefs.el
> bigger and increase the size of the dumped image.
> The comment in the file says that the autoload is needed for XEmacs, but
> that's not a good reason to keep it in the Emacs CVS, it's a one line
> change to a file that almost never changes.
> OK to remove the autoload line?
Actually, it reminds me that I faced a similar situation recently.
I did remove the autoload cookie, but in retrospect I think it was the
wrong thing to do. We should instead teach our build system to skip
preloaded files when creating the loaddefs.el file.
Stefan
Stefan Monnier
Dan Nicolaescu
I got rid of most of the autoloads in the preloaded files a couple of
years ago, this must have fallen through the cracks.
Skipping register.el while creating loaddefs.el would be wrong, nothing
else would create the register bindings then.
And there's 3 autoloads more in composite.el, that's all the autoloads
in the unconditionally preloaded files.
Given that there's so few autoloads left, it does not seem like a good
idea to change the build system instead of just removing them.
More, for the specific case of ediff-hook.el, IMO the best thing to do
is got fold the emacs specific contents of that file into menu-bar.el
and get rid of it. Almost all of the menu creation happens in menu-bar.el...
Glenn Morris
> The ###autoload in ediff-hook.el is not needed
[...]
> OK to remove the autoload line?
The last time this question was asked, the ediff maintainer asked for
it not to be removed:
http://lists.gnu.org/archive/html/emacs-devel/2008-03/msg00896.html
Dan Nicolaescu
Hmm, I forgot about this.
But removing a single line in a file that hardly ever changes does not
seem like a bit maintenance burden.
On the other hand the autoload adds ~10KB to all emacs downloads, plus a
few symbols that are defined and never ever used (but need to be GCed
every single time for ALL users).
Stefan Monnier
>> [...]
>> > OK to remove the autoload line?
>>
>> The last time this question was asked, the ediff maintainer asked for
>> it not to be removed:
>>
>> http://lists.gnu.org/archive/html/emacs-devel/2008-03/msg00896.html
> Hmm, I forgot about this.
> But removing a single line in a file that hardly ever changes does not
> seem like a bit maintenance burden.
That's only because Michael is the only maintainer who stood strong.
Many other preloaded files would be happy to keep
their ;;;###autoload cookies. They're a form of documentation and make
it easier to switch back&forth between preloaded and non-preloaded.
> On the other hand the autoload adds ~10KB to all emacs downloads, plus a
> few symbols that are defined and never ever used (but need to be GCed
> every single time for ALL users).
That's why those cookies should be skipped for preloaded files.
Stefan
Glenn Morris
> That's why those cookies should be skipped for preloaded files.
Any ideas on how to achieve this?
The list of preloaded files is not readily obtainable until after
emacs has been dumped, and dumping requires loaddefs.el, so it's kind
of circular.
I can only think of something cheesy, like evaling loadup.el with
`load' temporarily redefined.
Stefan Monnier
> Any ideas on how to achieve this?
> The list of preloaded files is not readily obtainable until after
> of circular.
Luckily, ldefs-boot.el saves the day: Tada!
Stefan
Glenn Morris
> Luckily, ldefs-boot.el saves the day: Tada!
Huh, for some reason I thought ldefs-boot wasn't used anymore during
bootstrap, but it is. I thought this was therefore going to be
straightforward, but because loadup.el sets purify-flag nil in
bootstrap-emacs, when the autoloads are made, preloaded-file-list only
contains loadup.el.
Stefan Monnier
BTW, the list of preloaded files is already encoded in src/Makefile.in.
Or you could otherwise extract it statically with something like
sed -n -e 's/.*(load "\(.*\)".*/\1/p' lisp/loadup.el
-- Stefan
Dan Nicolaescu
This fails, it won't generate autoloads for emacs-lisp/easymenu.el, and
those autoloads are needed.
Stefan Monnier
>> > bootstrap, but it is. I thought this was therefore going to be
>> > straightforward, but because loadup.el sets purify-flag nil in
>> > bootstrap-emacs, when the autoloads are made, preloaded-file-list only
>> > contains loadup.el.
>> BTW, the list of preloaded files is already encoded in src/Makefile.in.
>> Or you could otherwise extract it statically with something like
>> sed -n -e 's/.*(load "\(.*\)".*/\1/p' lisp/loadup.el
> This fails, it won't generate autoloads for emacs-lisp/easymenu.el, and
> those autoloads are needed.
Then you want the list that's in src/Makefile.in, since it includes the
distinction between those files that are preloaded on all systems vs
those that are system-dependent.
Stefan
Glenn Morris
> Then you want the list that's in src/Makefile.in, since it includes the
> distinction between those files that are preloaded on all systems vs
> those that are system-dependent.
That's a good suggestion, thanks.
I found that setting preloaded-file-list in bootstrap-emacs does not
work anyway. At that stage files are being loaded uncompiled, so extra
files (eg cl) end up being added to preloaded-file-list due to
eval-when-compile.
Anyway, here's an attempt at your suggestion. The final version
probably should not print the "skipped" message.
Index: Makefile.in
===================================================================
RCS file: /sources/emacs/emacs/lisp/Makefile.in,v
retrieving revision 1.193
diff -c -c -w -r1.193 Makefile.in
*** Makefile.in 16 Sep 2009 03:10:17 -0000 1.193
--- Makefile.in 26 Sep 2009 19:16:12 -0000
***************
*** 131,143 ****
# The chmod +w is to handle env var CVSREAD=1. Files named
# are identified by being the value of `generated-autoload-file'.
! autoloads: $(LOADDEFS) doit
chmod +w $(lisp)/ps-print.el $(lisp)/emulation/tpu-edt.el \
$(lisp)/emacs-lisp/cl-loaddefs.el $(lisp)/mail/rmail.el \
$(lisp)/dired.el $(lisp)/ibuffer.el
wd=$(lisp); $(setwins_almost); \
echo Directories: $$wins; \
! $(emacs) -l autoload --eval '(setq generated-autoload-file "$(lisp)/loaddefs.el")' -f batch-update-autoloads $$wins
# This is required by the bootstrap-emacs target in ../src/Makefile, so
# we know that if we have an emacs executable, we also have a subdirs.el.
--- 131,146 ----
# The chmod +w is to handle env var CVSREAD=1. Files named
# are identified by being the value of `generated-autoload-file'.
! # The Makefile dependency is to make any missing-file error more explicit.
! autoloads: $(LOADDEFS) ../src/Makefile doit
chmod +w $(lisp)/ps-print.el $(lisp)/emulation/tpu-edt.el \
$(lisp)/emacs-lisp/cl-loaddefs.el $(lisp)/mail/rmail.el \
$(lisp)/dired.el $(lisp)/ibuffer.el
wd=$(lisp); $(setwins_almost); \
echo Directories: $$wins; \
! preload=`sed -n -e '/^lisp=/ s/$${lispsource}//g p' ../src/Makefile | \
! sed 's/^lisp= //'`; \
! $(emacs) -l autoload --eval "(setq generated-autoload-file \"$(lisp)/loaddefs.el\" autoload-excludes \"$${preload}\")" -f batch-update-autoloads $$wins
# This is required by the bootstrap-emacs target in ../src/Makefile, so
# we know that if we have an emacs executable, we also have a subdirs.el.
Index: autoload.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/emacs-lisp/autoload.el,v
retrieving revision 1.143
diff -c -c -w -r1.143 autoload.el
*** autoload.el 5 Jan 2009 03:20:44 -0000 1.143
--- autoload.el 26 Sep 2009 19:16:22 -0000
***************
*** 58,63 ****
--- 58,66 ----
read and an autoload made for it. If there is further text on the line,
that text will be copied verbatim to `generated-autoload-file'.")
+ (defvar autoload-excludes nil
+ "If non-nil, list of absolute file names not to scan for autoloads.")
+
(defconst generate-autoload-section-header "\f\n;;;### "
"String that marks the form at the start of a new file's autoload section.")
***************
*** 347,353 ****
relfile
;; nil until we found a cookie.
output-start)
!
(with-current-buffer (or visited
;; It is faster to avoid visiting the file.
(autoload-find-file file))
--- 350,357 ----
relfile
;; nil until we found a cookie.
output-start)
! (if (member absfile autoload-excludes)
! (message "Generating autoloads for %s...skipped" file)
(with-current-buffer (or visited
;; It is faster to avoid visiting the file.
(autoload-find-file file))
***************
*** 452,458 ****
(message "Generating autoloads for %s...done" file))
(or visited
;; We created this buffer, so we should kill it.
! (kill-buffer (current-buffer))))
;; If the entries were added to some other buffer, then the file
;; doesn't add entries to OUTFILE.
(or (not output-start) otherbuf))))
--- 456,462 ----
(message "Generating autoloads for %s...done" file))
(or visited
;; We created this buffer, so we should kill it.
! (kill-buffer (current-buffer)))))
;; If the entries were added to some other buffer, then the file
;; doesn't add entries to OUTFILE.
(or (not output-start) otherbuf))))
***************
*** 649,654 ****
--- 653,670 ----
(defun batch-update-autoloads ()
"Update loaddefs.el autoloads in batch mode.
Calls `update-directory-autoloads' on the command line arguments."
+ ;; For use during the Emacs build process only. We do the file-name
+ ;; expansion here rather than in lisp/Makefile in order to keep the
+ ;; shell command line short. (Long lines are an issue on some systems.)
+ (if (stringp autoload-excludes)
+ (setq autoload-excludes
+ (mapcar
+ (lambda (file)
+ (concat
+ (expand-file-name (file-name-sans-extension file)
+ (file-name-directory generated-autoload-file))
+ ".el"))
+ (split-string autoload-excludes))))
(let ((args command-line-args-left))
(setq command-line-args-left nil)
(apply 'update-directory-autoloads args)))
Stefan Monnier
> work anyway. At that stage files are being loaded uncompiled, so extra
> files (eg cl) end up being added to preloaded-file-list due to
> eval-when-compile.
> Anyway, here's an attempt at your suggestion. The final version
> probably should not print the "skipped" message.
That looks pretty good. There's still a "minor" problem that
lisp/makefile.w32 also needs update.
BTW, now that I think about it: is it really necessary to skip those
files? I mean, what is the impact of having such redundant autoloads in
loaddefs.el?
Stefan
Dan Nicolaescu
From memory as I can't verify at the moment: the download size of the
emacs source is increased by > 10K, and the binary is 10KB bigger.
All of that is due to the single autoload line in ediff-hook.el, so the
can be solved by just removing that single line, or by killing the
ediff-hook.el file and moving the emacs specific code in menu-bar.el
(where it logically belongs anyway).
Stefan Monnier
>> files? I mean, what is the impact of having such redundant autoloads in
>> loaddefs.el?
>> From memory as I can't verify at the moment: the download size of the
> emacs source is increased by > 10K, and the binary is 10KB bigger.
I understand where the 10K of download-size comew from, but do you know
why it also impacts the final binary size? I mean: the objects and
variables created by these redundant autoloads should be overwritten by
the subsequent load of the actual files, so the only impact they should
have is if the GC ends up unable to reuse the corresponding memory cells
(and unable to return them to the system), right? Or are these
chunks of autoload-data placed in the pure-space?
Stefan
Dan Nicolaescu
Sorry, I don't know.
Another thing that I observed (and hope not to forget until I can
investigate properly): a lot of free conses and strings appear after the
"finding pointers to doc strings" phase in loadup.el.
It seems that we might have to try to avoid allocating and setting the
doc strings while dumping.
Emacs bug Tracking System
with message-id <w7iqetx...@fencepost.gnu.org>
and subject line Re: bug#4446: remove ###autoload from ediff-hook.el
has caused the Emacs bug report #4446,
regarding remove ###autoload from ediff-hook.el
to be marked as done.
This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.
(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact ow...@emacsbugs.donarmstrong.com
immediately.)
--
4446: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=4446
Emacs Bug Tracking System
Contact ow...@emacsbugs.donarmstrong.com with problems