bug#30491: 26.0; (elisp) `Profiling'

bug#30491: 26.0; (elisp) `Profiling'

[Drew Adams]

1. "how much resource" is not good English.  How much of a given
   resource or how many resources, not how much resource.  "Resource" is
   a countable noun.

2. "See the functions 'benchmark-run' and 'benchmark-run-compiled'".
   Those are macros, not functions.  And it should probably mention
   command `benchmark'.

3. The node should say something about library `elp', or it should not
   mention it.  The approach should be similar to what is said about
   library `benchmark.el', at a minimum.

4. A simple example of using the profiler commands would be welcome.


In GNU Emacs 26.0.91 (build 1, x86_64-w64-mingw32)
 of 2018-01-22
Repository revision: 752fba992b793a74d202c9cfc3e1a92fd458e748
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install 'CFLAGS=-O2 -static -g3''

bug#30491: 26.0; (elisp) `Profiling'

[Drew Adams]

Also, some brief comparison of profile, elp, and benchmark could be helpful: what are their particular purposes, strengths, etc.

bug#30491: 26.0; (elisp) `Profiling'

[Eli Zaretskii]

> Date: Fri, 16 Feb 2018 10:11:59 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> 
> 1. "how much resource" is not good English.  How much of a given
>    resource or how many resources, not how much resource.  "Resource" is
>    a countable noun.
> 
> 2. "See the functions 'benchmark-run' and 'benchmark-run-compiled'".
>    Those are macros, not functions.  And it should probably mention
>    command `benchmark'.

I fixed those.

> 3. The node should say something about library `elp', or it should not
>    mention it.  The approach should be similar to what is said about
>    library `benchmark.el', at a minimum.

Done.

> 4. A simple example of using the profiler commands would be welcome.

Not sure what example you had in mind.  The text mentions 3 profiler
commands to invoke, and describes the resulting profile.  What would
the example include, besides just "M-x profile-start" etc., which
sounds redundant?

> Also, some brief comparison of profile, elp, and benchmark could be helpful: what are their particular purposes, strengths, etc.

I think this was already clear from the text: e.g., benchmark only
measures the total execution time of a form, it doesn't produce any
insights on the distribution of that time between the form's
components.  I added more text to make some differences more clear.

Thanks.

bug#30491: 26.0; (elisp) `Profiling'

[Drew Adams]

> > 4. A simple example of using the profiler commands would be welcome.
> 
> Not sure what example you had in mind.  The text mentions 3 profiler
> commands to invoke, and describes the resulting profile.  What would
> the example include, besides just "M-x profile-start" etc., which
> sounds redundant?

I guess I was mainly thinking of use of the resulting output.

Perhaps it was not so much a doc problem as a problem of
usability of the UI of the report.

I know I was looking for a way to "open" not just the entry
on the current line, but also a way to open either (1) all
entries or (2) all entries in the region.  I ended up using
a keyboard macro to do #1.

And I found that the action that is now associated with a
`mouse-1' click, which is to "open" the entry of the line
clicked, is more annoying than helpful.  Clicking to set
point is too useful to be lost or relegated to "slow"
clicking.  I ended up using only the keyboard to move the
cursor, which isn't always ideal for direct access.

Yes, those are not comments about the doc.  Perhaps they
could be considered anyway, without needing a separate
bug report?

> > Also, some brief comparison of profile, elp, and benchmark could be
> helpful: what are their particular purposes, strengths, etc.
> 
> I think this was already clear from the text: e.g., benchmark only
> measures the total execution time of a form, it doesn't produce any
> insights on the distribution of that time between the form's
> components.  I added more text to make some differences more clear.

This post makes a start, but it doesn't mention elp.
Perhaps the text you've added (I've not seen it yet)
already makes a similar brief comparison.

https://emacs.stackexchange.com/a/540/105

Thanks for the doc improvements.

bug#30491: 26.0; (elisp) `Profiling'

[Noam Postavsky]

Drew Adams <drew.adams@oracle.com> writes:

> I know I was looking for a way to "open" not just the entry
> on the current line, but also a way to open either (1) all
> entries or (2) all entries in the region.  I ended up using
> a keyboard macro to do #1.

You can use the prefix argument to profiler-report-toggle-entry (C-u
RET).  This is documented as of Emacs 25 (see [1: 272391f1d6] and
Bug#13455).

[1: 272391f1d6]: 2016-07-28 20:02:42 -0400
  profiler: document prefix arg for tree expansion
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=272391f1d63eae0c1002d62fa5e5b1d63a0da767

bug#30491: 26.0; (elisp) `Profiling'

[Eli Zaretskii]

> Date: Sat, 17 Feb 2018 07:45:34 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 30491@debbugs.gnu.org
> 
> > > 4. A simple example of using the profiler commands would be welcome.
> > 
> > Not sure what example you had in mind.  The text mentions 3 profiler
> > commands to invoke, and describes the resulting profile.  What would
> > the example include, besides just "M-x profile-start" etc., which
> > sounds redundant?
> 
> I guess I was mainly thinking of use of the resulting output.
> 
> Perhaps it was not so much a doc problem as a problem of
> usability of the UI of the report.
> 
> I know I was looking for a way to "open" not just the entry
> on the current line, but also a way to open either (1) all
> entries or (2) all entries in the region.  I ended up using
> a keyboard macro to do #1.

It's much easier to do "C-u RET" on the top line of the call-tree.
And the fact that prefix argument has this effect is documented.

> > > Also, some brief comparison of profile, elp, and benchmark could be
> > helpful: what are their particular purposes, strengths, etc.
> > 
> > I think this was already clear from the text: e.g., benchmark only
> > measures the total execution time of a form, it doesn't produce any
> > insights on the distribution of that time between the form's
> > components.  I added more text to make some differences more clear.
> 
> This post makes a start, but it doesn't mention elp.
> Perhaps the text you've added (I've not seen it yet)
> already makes a similar brief comparison.
> 
> https://emacs.stackexchange.com/a/540/105

I think this is covered now.

bug#30491: 26.0; (elisp) `Profiling'

[Drew Adams]

> And I found that the action that is now associated with a
> `mouse-1' click, which is to "open" the entry of the line
> clicked, is more annoying than helpful.  Clicking to set
> point is too useful to be lost or relegated to "slow"
> clicking.  I ended up using only the keyboard to move the
> cursor, which isn't always ideal for direct access.

I have `mouse-1-click-follows-link' = nil, which normally
means that `mouse-1' click just sets point.  At the very
least it normally does NOT mean that `mouse-1' click does
the same thing that `mouse-2' click does.

In this context, I would like `mouse-2' to do what `mouse-1'
is currently doing: open the clicked entry.  And I would
like (and expect) that `mouse-1' simply sets point, as
usual.

This change to making `mouse-1' always open the entry
seems to go against Emacs convention and subvert the
user's setting of `mouse-1-click-follows-link'.

Not only that, but the normal behavior of non-nil
`mouse-1-click-follows-link' is that if you hold `mouse-1'
depressed for a moment the command simply sets point.
But in the profiler report buffer doing that instead
shows `C-h f' output for the function.

So it seems that the Profiler report key bindings do
not respect `mouse-1-click-follows-link' for either
a nil or a non-nil value.

This is bad design, IMO - a design bug.  It is pretty
much impossible, it seems, to use `mouse-1' to set
point.

bug#30491: 26.0; (elisp) `Profiling'

[Drew Adams]

> Not only that, but the normal behavior of non-nil
> `mouse-1-click-follows-link' is that if you hold `mouse-1'
> depressed for a moment the command simply sets point.
> But in the profiler report buffer doing that instead
> shows `C-h f' output for the function.
> 
> So it seems that the Profiler report key bindings do
> not respect `mouse-1-click-follows-link' for either
> a nil or a non-nil value.
> 
> This is bad design, IMO - a design bug.  It is pretty
> much impossible, it seems, to use `mouse-1' to set
> point.

My bad about that part.  Holding `mouse-1' depressed
does not show help on the function.  But I was right
that it also does not set point (as it should, if
`mouse-1-click-follows-link' were respected).

Holding `mouse-1' for a moment does nothing different.
The slow click just toggles the entry, same as a fast
click.

---

I was confused here by reading the help for `C-h k'
for the `mouse-1' click.  At the end of that help I
read this:

  For documentation of the corresponding mouse down
  event <down-mouse-1>, click and hold the mouse button
  longer than 0.5 second(s).

I was misreading that as applying to the command whose
doc I was reading: `profiler-report-toggle-entry'.

Instead, that sentence is about the key action I used
at the prompt of `C-h k'.

I think this help text is unclear.  99% of the text
is about a COMMAND - the command bound to the mouse
up event.  And then, with no separation at all, we
read a sentence that is suddenly about the second
event you caused in response to the `C-h k' prompt'.

This is awful (IMHO).  At the very least, that
sentence should be set off with a clear separator
(e.g. horizontal line or C-l char), and it should
be in parentheses.

Some better help design is needed for this, in any
case.  I'm sure I won't be the last user to fall
prey to this design gotcha.

bug#30491: 26.0; (elisp) `Profiling'

[Drew Adams]

> > I know I was looking for a way to "open" not just the entry
> > on the current line, but also a way to open either (1) all
> > entries or (2) all entries in the region.  I ended up using
> > a keyboard macro to do #1.

EZ> It's much easier to do "C-u RET" on the top line of the call-tree.
EZ> And the fact that prefix argument has this effect is documented.

NP> You can use the prefix argument to profiler-report-toggle-entry (C-u
NP> RET).  This is documented as of Emacs 25 (see [1: 272391f1d6] and
NP> Bug#13455).

Yes, I see that now.  I don't think it's very obvious, though.

For some reason, we've dedicated all of these keys to this
command: `mouse-1', `i', `TAB', `RET', and we've put it in
the menu-bar menu.  (Why so many keys?)

And yet we've dedicated no key to doing directly what `C-u i'
does.  Maybe that should be rethought?

At the very least, maybe that action should be added to the
menu-bar menu, so that users can more easily discover that
they can invoke it using `C-u i'.

(Ccing Michael H.)

bug#30491: 26.0; (elisp) `Profiling'

[Drew Adams]

Other UX problems I see with `profiler-report-mode'.  #1 is the
greatest annoyance, to me.

1. There should be an easy way to expand the tree space horizontally,
   to eliminate eliding (`...').  When I use the profiler the code
   that I really want to see is too often completely missing - it's
   hidden by `...'.  How to show it?

   To work around this limitation I found myself tweaking variables
   `profiler-report-(cpu|memory)-line-format' to increase the `left'
   value, which is ridiculous.

   Is there a better way to expand/expose the elided entries?
   I was hoping that maybe command `profiler-report-expand-entry'
   would do the job, but it doesn't.  Expanding for that command
   means exposing but not widening the field (column) so that an
   elided entry is shown.

2. Command `profiler-report-expand-entry' with a prefix arg does
   not do what its doc string says - it does not expand the whole
   subtree.  It expands the subtree at point only if the current
   line has a `+' sign.  So if the current line has `-' instead,
   and if some lines under it (in its subtree) have `+', those
   `+' lines are not expanded.

   I think it should do what its doc says, or at least that some
   command should do that.

3. Commands `profiler-find-profile(-other-(window|frame))' should
   maybe be bound to keys.  They complement function
   `profiler-report-write-profile'.  They should at least be added
   to menu-bar menu `Profiler'.  They let you move to a different
   (saved) profiler report.

bug#30491: 26.0; (elisp) `Profiling'

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> > > Also, some brief comparison of profile, elp, and benchmark could be
>> > helpful: what are their particular purposes, strengths, etc.
>> >
>> > I think this was already clear from the text: e.g., benchmark only
>> > measures the total execution time of a form, it doesn't produce any
>> > insights on the distribution of that time between the form's
>> > components.  I added more text to make some differences more clear.
>>
>> This post makes a start, but it doesn't mention elp.
>> Perhaps the text you've added (I've not seen it yet)
>> already makes a similar brief comparison.
>>
>> https://emacs.stackexchange.com/a/540/105
>
> I think this is covered now.

From reading this thread, it seems like the documentation issues here
have been fixed.  There were also some tangents, but separate bug
reports were filed for those.  So it doesn't seem like there's nothing
more to do here, and I'm closing this bug.

As always, please report back if there's something I missed.  Please
open new bug reports for any remaining issues that are not related to
the documentation changes discussed here.  Thanks in advance.

bug#30491: [External] : Re: bug#30491: 26.0; (elisp) `Profiling'

[Drew Adams]

> From reading this thread, it seems like the documentation issues here
> have been fixed.  There were also some tangents, but separate bug
> reports were filed for those.  So it doesn't seem like there's nothing
> more to do here, and I'm closing this bug.
> 
> As always, please report back if there's something I missed.  Please
> open new bug reports for any remaining issues that are not related to
> the documentation changes discussed here.  Thanks in advance.

Some doc problems were addressed; other problems (UI)
were not.  (The bug wasn't closed in 2018, BTW.)

bug#30491: [External] : Re: bug#30491: 26.0; (elisp) `Profiling'

[Stefan Kangas]

Drew Adams <drew.adams@oracle.com> writes:

>> From reading this thread, it seems like the documentation issues here
>> have been fixed.  There were also some tangents, but separate bug
>> reports were filed for those.  So it doesn't seem like there's nothing
>> more to do here, and I'm closing this bug.
>>
>> As always, please report back if there's something I missed.  Please
>> open new bug reports for any remaining issues that are not related to
>> the documentation changes discussed here.  Thanks in advance.
>
> Some doc problems were addressed; other problems (UI)
> were not.  (The bug wasn't closed in 2018, BTW.)

The fact that the bug wasn't closed doesn't tell us much, as bugs are
routinely left open for over a decade around here just because someone
forgot to close it.  ;-)

If I understand you correctly, you are saying that there are outstanding
UI issues left to fix.  I suggest you open new bug reports for them, as
it takes a significant amount of time to trawl through bug reports about
unrelated issues looking for such nuggets of information.  As is
evidenced by me closing this bug: I didn't see that there was anything
in here that wasn't already fixed, even after having read the discussion
in full.

Thanks in advance.

bug#30491: [External] : Re: bug#30491: 26.0; (elisp) `Profiling'

[Drew Adams]

It's OK with me that the bug be closed.  If someone's
interested in improving the UI then the discussion in
the bug thread might help.  If no one's interested,
so be it.

bug#30491: [External] : Re: bug#30491: 26.0; (elisp) `Profiling'

[Eli Zaretskii]

> From: Drew Adams <drew.adams@oracle.com>
> CC: "30491-done@debbugs.gnu.org" <30491-done@debbugs.gnu.org>
> Date: Sat, 23 Oct 2021 18:18:56 +0000
> 
> > From reading this thread, it seems like the documentation issues here
> > have been fixed.  There were also some tangents, but separate bug
> > reports were filed for those.  So it doesn't seem like there's nothing
> > more to do here, and I'm closing this bug.
> > 
> > As always, please report back if there's something I missed.  Please
> > open new bug reports for any remaining issues that are not related to
> > the documentation changes discussed here.  Thanks in advance.
> 
> Some doc problems were addressed; other problems (UI)
> were not.  (The bug wasn't closed in 2018, BTW.)

The UI design is an entirely separate issue.

The rest was fixed.

bug#30491: [External] : Re: bug#30491: 26.0; (elisp) `Profiling'

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sat, 23 Oct 2021 11:36:37 -0700
> Cc: "30491-done@debbugs.gnu.org" <30491-done@debbugs.gnu.org>
> 
> The fact that the bug wasn't closed doesn't tell us much, as bugs are
> routinely left open for over a decade around here just because someone
> forgot to close it.  ;-)

That someone didn't forget.  Bugs opened by Drew were routinely left
open, because Drew used to vociferously oppose their being closed,
still does.

bug#30491: [External] : Re: bug#30491: 26.0; (elisp) `Profiling'

[Drew Adams]

> That someone didn't forget.  Bugs opened by Drew were routinely left
> open, because Drew used to vociferously oppose their being closed,
> still does.

Nonsense and FUD.  And still you persist with the ad
hominem...

Bugs I've filed get closed all the time, and most
closures I express no objection to, including when
I think the bug is a bug and should/could be fixed.

It's _certainly_ not the case that bugs I filed have
been left open simply because I've opposed closing.
Objections from me have stopped closure only when
someone was convinced of a good reason not to close.

What's true is that I've filed a lot of bugs.  And
you're welcome.

bug#30491: [External] : Re: bug#30491: 26.0; (elisp) `Profiling'

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> The fact that the bug wasn't closed doesn't tell us much, as bugs are
>> routinely left open for over a decade around here just because someone
>> forgot to close it.  ;-)
>
> That someone didn't forget.

Oh, I wasn't trying to single anyone out.  I should probably have said
"nobody remembered to close it" to more accurately convey my joke
(assuming it was at all worth conveying).

bug#30491: [External] : Re: bug#30491: 26.0; (elisp) `Profiling'

[Eli Zaretskii]

> From: Drew Adams <drew.adams@oracle.com>
> CC: "30491-done@debbugs.gnu.org" <30491-done@debbugs.gnu.org>
> Date: Sat, 23 Oct 2021 19:38:06 +0000
> 
> > That someone didn't forget.  Bugs opened by Drew were routinely left
> > open, because Drew used to vociferously oppose their being closed,
> > still does.
> 
> Nonsense and FUD.  And still you persist with the ad
> hominem...

As long as you keep thinking that the problem is with others, not with
your own conduct, you shouldn't be surprised that your opinions aren't
treated seriously.