Re: 28.0.50; Proposal: slightly more efficient package-quickstart.el

[Arthur Miller]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Arthur Miller [2021-07-20 02:27:26] wrote:
>> Since quickstart file is created by iterating through each and one
>> installed package whose path is already known, this calculation can be
>> performed once at generation time, rather than every time when Emacs
>> starts.
>
> It's indeed an optimization we can perform.  I'm not sure it's worth the
> extra code, tho, since there's always a risk it will break something.
>
> Have you made some measurements to see if it the difference is
> noticeable?

Not so much, but it is not so much about noticable difference, more
about not performing unnecessary computation. As the loop progresses,
for every dir N, the loop does N-1 path comparisons. These are
unnecessary comparisons since when we know for sure that the string is
not added yet. Trend in Emacs seems to go towards having more and more
packages installed. This removes N(N-1)) comparisons.

>> -        (Info-directory-list '("")))
>> +        (Info-directory-list '(""))
>> +        paths)
>
> The GNU coding standard would call this `dirs` since a "path" is a list
> of directories used for searching.  I guess you could also call it
> `path` (singular).
Ok.
>> @@ -4164,6 +4166,10 @@
>>                (replace-match (if (match-end 1) "" pfile) t t)))
>>            (unless (bolp) (insert "\n"))
>>            (insert ")\n")))
>> +      (goto-char (point-min))
>> +      (while (re-search-forward "^(add-to-list.*load-path" nil t)
>> +        (goto-char (line-beginning-position))
>> +        (kill-sexp))
>>        (pp `(setq package-activated-list
>>                   (append ',(mapcar #'package-desc-name 
>> package--quickstart-pkgs)
>>                           package-activated-list))
>
> Beside the hypothetical risk that the regexp matches an `add-to-list` to
> an unrelated variable, I think this risks introducing real bugs for
> packages which use (add-to-list 'load-path <subdir>) to add
> *sub*directories.
Yes, I was aware of it, but I am not sure how to write the regex, to
avoid that risk. Maybe I could check if the path refers to itself.

>> @@ -4175,6 +4181,10 @@
>>                        (setq Info-directory-list
>>                              (append ',info-dirs Info-directory-list)))
>>                (current-buffer))))
>> +      (goto-char (point-min))
>> +      (forward-line 3)
>> +      (insert (concat "\n(nconc load-path '" (prin1-to-string paths) ")\n"))
>> +      (goto-char (point-max))
>
> This inserts the dirs at the end of `load-path` whereas `add-to-list`
> usually adds them to the beginning.  I think this will break configs
> where a newer version of Org is installed (because the built-in Org
> will then take priority).
Ok. I didn't notice anything on my own computer, but it says nothing
about other people setups. Since nconc has to loop throug entire list to
append to it's tail, I thought it is less to loop through existing
load-path, since it probably contains only Emacs lisp directories at
that point. But I can exchange the order.

> Of course, we can fix those problems, but unless there's a compelling
> performance argument, I think it's a waste of time.

It's quite simple to do, it's not much of the code, Performance here is
probably not spending cpu and saving battery life. I don't know, at
least in theory :).