Interesting! Yes, there is redundant work happening when styling field-by-field. Hmmm... one could add some kind cache shared between all "inspector fields" in an inspector. That stepping is not the issue here because debugger inspectors do not step.

Best,
Marcel
Am 03.10.2020 00:07:35 schrieb Thiede, Christoph <christoph.thiede at student.hpi.uni-potsdam.de>:
Hi Tim,

I added the styling to the inspector fields because, in my opinion, this helps you to identify different kinds of fields faster. But we should start honoring the preference SHTextStylerST80 syntaxHighlightingAsYouType again, this would allow you to turn off styling in your individual image.

> And if one *does* feel a need for that, since we know quite a lot about what the field labels are, why can one not do some more focussed action than a general 'err, here, style this thing for me'?

This was simply a question of code quality vs. optimization. One main objective of the recent inspector refactoring was to make it easier to extend/subclass it, and reproducing the styler logic would make this unnecessarily complicated. Also, the styling must honor be updated when the user interface theme is changed, so using Shout out of the box simply looked like the most straightforward and elegant solution to me ...

We could also introduce a cache of the styled field titles in Inspector >> #resetFields by computing a hash value from the list of unstyled field titles.

Best,
Christoph
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von tim Rowledge <tim at rowledge.org>
Gesendet: Freitag, 2. Oktober 2020 23:37:02
An: The general-purpose Squeak developers list
Cc: packages at lists.squeakfoundation.org
Betreff: Re: [squeak-dev] The Trunk: Tools-eem.994.mcz
 
Forgive the potentially dumb question, but why exactly does one want to use any sort of styling stuff on fields in an inspector?

And if one *does* feel a need for that, since we know quite a lot about what the field labels are, why can one not do some more focussed action than a general 'err, here, style this thing for me'?

> On 2020-10-02, at 2:18 PM, Thiede, Christoph <Christoph.Thiede at student.hpi.uni-potsdam.de> wrote:
>
> Hi Eliot,
>
> thanks for the exact steps to reproduce. I will use [1000 timesRepeat: [self performAction]] timeProfile because I don't have the AndreasSystemProfiler.
>
> Does ShoutCore-ct.78 from the inbox help you to speed things up? For me, it speeds up the call from 3.84 sec down to 1.07 sec - but still, there happens a lot of redundant shout styling.
> Hm, why can't we recreate the fields lazily in #fields instead of doing this eagerly in #resetFields? This is how I originally implemented it but then Marcel changed it - I don't know the reason. :-)
>
> Best,
> Christoph
> Von: Eliot Miranda <eliot.miranda at gmail.com>
> Gesendet: Freitag, 2. Oktober 2020 22:59:04
> An: The general-purpose Squeak developers list; Taeumel, Marcel; Thiede, Christoph
> Cc: packages at lists.squeakfoundation.org
> Betreff: Re: [squeak-dev] The Trunk: Tools-eem.994.mcz
> 
> Hi Marcel, Hi Christoph,
>
>     I emailed the list and cc'ed you to get your attention.  Forgive my rude interruption.  I finally found out where the slow down really is.  It is in
>
> Inspector>>fieldList
> "Return a list of texts that identify the fields for the object under inspection so that the user can make an informed decision on what to inspect."
> ^ self fieldListStyler
> ifNil: [self fields collect: [:field | field name]]
> ifNotNil: [:styler |
> self updateStyler: styler.
> self fields collect: [:field |
> field shouldStyleName
> ifTrue: [styler styledTextFor: field name asText]
> ifFalse: [field name]]]
>
> So this runs a styler over the entire method every time one steps.  And if one is stepping through a doit it will call the decompiler to generate the source to style, every time you step.  We have to do better :-)
>
> Here's how to profile it.  Debug a doit.  I wrote this one:
>
> | t |
> t := 0.
> [| a b c |
> a := 1. b := 2. c := 100.
> (a = 1 and: [b = 2 and: [c = 100]])
> ifTrue:
> [1 to: 100 by: 2 do:
> [:i| t := t + 1]]
> ifFalse:
> [a to: c by: b do:
> [:i| t := t + 1]]] repeat.
> t
>
> Once in the debugger inspect the "Over" button.  Then in that inspector evaluate
>
>       AndreasSystemProfiler spyOn: [1000 timesRepeat: [self performAction]]
>
> and you'll see that essentially all the time is going into SHTextStylerST80(SHTextStyler) styledTextFor:
>
> On Fri, Oct 2, 2020 at 1:44 PM <commits at source.squeak.org> wrote:
> Eliot Miranda uploaded a new version of Tools to project The Trunk:
> http://source.squeak.org/trunk/Tools-eem.994.mcz [http://source.squeak.org/trunk/Tools-eem.994.mcz]
>
> ==================== Summary ====================
>
> Name: Tools-eem.994
> Author: eem
> Time: 2 October 2020, 1:44:29.015648 pm
> UUID: 23145257-280d-4a4a-a4ba-3b5cc367f9a9
> Ancestors: Tools-eem.993
>
> Oops! Compiledmethod>>blockExtentsToTempsMap needs to observe that startKeysToBlockExtents has moved to DebuggerMethodMap.
>
> =============== Diff against Tools-eem.993 ===============
>
> Item was changed:
>   ----- Method: CompiledMethod>>blockExtentsToTempsMap (in category '*Tools-Debugger-support') -----
>   blockExtentsToTempsMap
>         "If the receiver has been copied with temp names answer a
>          map from blockExtent to temps map in the same format as
>          BytecodeEncoder>>blockExtentsToTempNamesMap.  if the
>          receiver has not been copied with temps answer nil."
>         ^self holdsTempNames ifTrue:
> +               [self mapFromBlockKeys: (self debuggerMap startKeysToBlockExtents values sort: [:assocA :assocB| assocA first <= assocB first])
> -               [self mapFromBlockKeys: (self startKeysToBlockExtents values sort: [:assocA :assocB| assocA first <= assocB first])
>                         toSchematicTemps: self tempNamesString]!
>
>
>
>
> --
> _,,,^..^,,,_
> best, Eliot


tim
--
tim Rowledge; tim at rowledge.org; http://www.rowledge.org/tim [http://www.rowledge.org/tim]
Do files get embarrassed when they get unzipped?



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20201005/ad960e03/attachment.html>