GNU bug report logs - #73533
[PATCH] Rewrite speedbar expansion for all descendants

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Morgan Willcock <morgan <at> ice9.digital>
Cc: 73533 <at> debbugs.gnu.org
Subject: bug#73533: [PATCH] Rewrite speedbar expansion for all descendants
Date: Sun, 29 Sep 2024 07:46:08 +0300

> From: Morgan Willcock <morgan <at> ice9.digital>
> Date: Sat, 28 Sep 2024 20:05:26 +0100
> 
> Attached is a patch which rewrites 'speedbar-expand-line-descendants'.
> 
> The previous version could get into an infinite loop by reaching the
> maximum recursion depth, although in practice the slow speed meant that
> most people would probably abort the operation before reaching that
> point.
> 
> The majority of the slowdown was because the motion commands being used
> were the variants which looked up information for every entry, displayed
> the information as a message, and adjusted the cursor position.  The
> messages were not readable because of being continually overwritten.
> 
> Here is a way to demonstrate that stack depth was increasing for items
> at the same level, that the messages were not readable, and how slow the
> whole process was:
> 
>   rm -rf /tmp/project
>   mkdir /tmp/project
>   for i in $(seq 1 50); do echo "(defun fun-$i ())" >> /tmp/project/file1.el; done
>   for i in $(seq 1 50); do echo "(defun fun-$i ())" >> /tmp/project/file2.el; done
>   emacs -Q \
>         --eval="(find-file \"/tmp/project/file1.el\")" \
>         --eval "(speedbar-get-focus)" \
>         --eval "(profiler-start 'cpu)" \
>         --eval "(speedbar-expand-line-descendants)" \
>         --eval "(profiler-stop)" \
>         --eval "(profiler-report)"
> 
> ...that should expand every entry in file1.el and not touch the entries
> in file2.el.
> 
> The replacement function is significantly faster.  Messages are only
> used to indicate that the function is running and when it is finished -
> the result is similar to manually clicking every node open.

Thanks.  Unfortunately, speedbar doesn't have a test suite, so I would
like to ask how you tested this rewrite, and whether we could have
some tests for this added to the test suite.

Regardless, it would be good to have both old and the new code
annotated with explanations of what each non-trivial line does, to
allow independent verification of the correctness of the rewrite by
people who are not familiar with speedbar code and don't immediately
understand the effects of a call to speedbar-naxt or
speedbar-restricted-move.

Also, this comment in the old code bothers me:

> -    ;; Now, inside the area expanded here, expand all subnodes of
> -    ;; the same descendant type.

What does it mean by "the same descendant type", and how does the old
and the new code make sure they expand only those descendants?

Previous Next

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