The Inbox: Morphic-ct.1771.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

The Inbox: Morphic-ct.1771.mcz

commits-2
A new version of Morphic was added to project The Inbox:
http://source.squeak.org/inbox/Morphic-ct.1771.mcz

==================== Summary ====================

Name: Morphic-ct.1771
Author: ct
Time: 25 May 2021, 7:38:48.072683 pm
UUID: 5a9ddaf0-a062-7047-b5b2-d2ae2da3fe15
Ancestors: Morphic-mt.1769

Fixes a bottleneck when opening a yellow button menu on a morph that contains a very large number of subsub*morphs. On not-so-fast systems, this can be reproduced using:

        self systemNavigation browseAllSelect: #notNil

On faster systems, you might need to write a small toolbuilder application to reproduce the bottleneck. I have an app with >10k list items in my image which actually blocked the image for several seconds when I yellow-clicked the window.

Fixed the problem without duplicating the logic of #allStringsAfter: by using a generator.

=============== Diff against Morphic-mt.1769 ===============

Item was changed:
  ----- Method: Morph>>addYellowButtonMenuItemsTo:event: (in category 'menu') -----
  addYellowButtonMenuItemsTo: aMenu event: evt
  "Populate aMenu with appropriate menu items for a  
  yellow-button (context menu) click."
  aMenu defaultTarget: self.
  ""
  Preferences noviceMode
  ifFalse: [aMenu addStayUpItem].
  ""
  self addModelYellowButtonItemsTo: aMenu event: evt.
  ""
  Preferences generalizedYellowButtonMenu
  ifFalse: [^ self].
  ""
  aMenu addLine.
  aMenu add: 'inspect' translated action: #inspect.
  ""
  aMenu addLine.
  self world selectedObject == self
  ifTrue: [aMenu add: 'deselect' translated action: #removeHalo]
  ifFalse: [aMenu add: 'select' translated action: #addHalo].
  ""
  (self isWorldMorph
  or: [self mustBeBackmost
  or: [self wantsToBeTopmost]])
  ifFalse: [""
  aMenu addLine.
  aMenu add: 'send to back' translated action: #goBehind.
  aMenu add: 'bring to front' translated action: #comeToFront.
  self addEmbeddingMenuItemsTo: aMenu hand: evt hand].
  ""
  self isWorldMorph
  ifFalse: [""
  Smalltalk
  at: #NCAAConnectorMorph
  ifPresent: [:connectorClass |
  aMenu addLine.
  aMenu add: 'connect to' translated action: #startWiring.
  aMenu addLine].
  ""
 
  self isFullOnScreen
  ifFalse: [aMenu add: 'move onscreen' translated action: #goHome]].
  ""
  Preferences noviceMode
  ifFalse: [""
  self addLayoutMenuItems: aMenu hand: evt hand.
  (owner notNil
  and: [owner isTextMorph])
  ifTrue: [self addTextAnchorMenuItems: aMenu hand: evt hand]].
  ""
  self isWorldMorph
  ifFalse: [""
  aMenu addLine.
  self addToggleItemsToHaloMenu: aMenu].
  ""
  aMenu addLine.
  self isWorldMorph
  ifFalse: [aMenu add: 'copy to paste buffer' translated action: #copyToPasteBuffer:].
+ (Generator on: [:gen | self streamAllStringsAfter: nil on: gen]) in: [:gen |
+ "optimized!! #allStringsAfter: can be slow for large subtrees."
+ gen atEnd ifFalse: [
+ aMenu add: 'copy text' translated action: #clipText]].
- (self allStringsAfter: nil) isEmpty
- ifFalse: [aMenu add: 'copy text' translated action: #clipText].
  ""
  self addExportMenuItems: aMenu hand: evt hand.
  ""
  (Preferences noviceMode not
  and: [self isWorldMorph not])
  ifTrue: [""
  aMenu addLine.
  aMenu add: 'adhere to edge...' translated action: #adhereToEdge].
  ""
  self addCustomMenuItems: aMenu hand: evt hand!

Item was changed:
  ----- Method: Morph>>allStringsAfter: (in category 'debug and other') -----
+ allStringsAfter: aSubmorph
- allStringsAfter: aSubmorph
- "return an OrderedCollection of strings of text in my submorphs.  If aSubmorph is non-nil, begin with that container."
 
+ ^ OrderedCollection streamContents: [:stream |
+ self streamAllStringsAfter: aSubmorph on: stream]!
- | list ok |
- list := OrderedCollection new.
- ok := aSubmorph isNil.
- self allMorphsDo:
- [:sub | | string |
- ok ifFalse: [ok := sub == aSubmorph]. "and do this one too"
- ok
- ifTrue:
- [(string := sub userString) ifNotNil:
- [string isString ifTrue: [list add: string] ifFalse: [list addAll: string]]]].
- ^list!

Item was added:
+ ----- Method: Morph>>streamAllStringsAfter:on: (in category 'debug and other') -----
+ streamAllStringsAfter: aSubmorph on: aStream
+ "Stream all strings of text in my submorphs on aStream. If aSubmorph is non-nil, begin with that container."
+
+ | ok |
+ ok := aSubmorph isNil.
+ self allMorphsDo: [:sub | | string |
+ ok ifFalse: [ok := sub == aSubmorph].
+ "and do this one too"
+ ok ifTrue: [
+ (string := sub userString)
+ ifNotNil: [string isString
+ ifTrue: [aStream nextPut: string]
+ ifFalse: [aStream nextPutAll: string]]]].!


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Morphic-ct.1771.mcz

marcel.taeumel
Hi Christoph.

Thanks. I think that the expected pattern is a little bit different, though. The explicit use of Generator is unfortunate but can be easily avoided. :-)

allStringsAfter: morph do: block
   "..."

allStringsAfter: morph
   ^ Array streamContents: [:stream |
      self allStringsAfter: morph do: [:string |
         stream nextPut: string]]

hasStringsAfter: morph
   self allStringsAfter: morph do: [:string | ^ true].
   ^ false

Best,
Marcel

Am 25.05.2021 19:39:07 schrieb [hidden email] <[hidden email]>:

A new version of Morphic was added to project The Inbox:
http://source.squeak.org/inbox/Morphic-ct.1771.mcz

==================== Summary ====================

Name: Morphic-ct.1771
Author: ct
Time: 25 May 2021, 7:38:48.072683 pm
UUID: 5a9ddaf0-a062-7047-b5b2-d2ae2da3fe15
Ancestors: Morphic-mt.1769

Fixes a bottleneck when opening a yellow button menu on a morph that contains a very large number of subsub*morphs. On not-so-fast systems, this can be reproduced using:

self systemNavigation browseAllSelect: #notNil

On faster systems, you might need to write a small toolbuilder application to reproduce the bottleneck. I have an app with >10k list items in my image which actually blocked the image for several seconds when I yellow-clicked the window.

Fixed the problem without duplicating the logic of #allStringsAfter: by using a generator.

=============== Diff against Morphic-mt.1769 ===============

Item was changed:
----- Method: Morph>>addYellowButtonMenuItemsTo:event: (in category 'menu') -----
addYellowButtonMenuItemsTo: aMenu event: evt
"Populate aMenu with appropriate menu items for a
yellow-button (context menu) click."
aMenu defaultTarget: self.
""
Preferences noviceMode
ifFalse: [aMenu addStayUpItem].
""
self addModelYellowButtonItemsTo: aMenu event: evt.
""
Preferences generalizedYellowButtonMenu
ifFalse: [^ self].
""
aMenu addLine.
aMenu add: 'inspect' translated action: #inspect.
""
aMenu addLine.
self world selectedObject == self
ifTrue: [aMenu add: 'deselect' translated action: #removeHalo]
ifFalse: [aMenu add: 'select' translated action: #addHalo].
""
(self isWorldMorph
or: [self mustBeBackmost
or: [self wantsToBeTopmost]])
ifFalse: [""
aMenu addLine.
aMenu add: 'send to back' translated action: #goBehind.
aMenu add: 'bring to front' translated action: #comeToFront.
self addEmbeddingMenuItemsTo: aMenu hand: evt hand].
""
self isWorldMorph
ifFalse: [""
Smalltalk
at: #NCAAConnectorMorph
ifPresent: [:connectorClass |
aMenu addLine.
aMenu add: 'connect to' translated action: #startWiring.
aMenu addLine].
""

self isFullOnScreen
ifFalse: [aMenu add: 'move onscreen' translated action: #goHome]].
""
Preferences noviceMode
ifFalse: [""
self addLayoutMenuItems: aMenu hand: evt hand.
(owner notNil
and: [owner isTextMorph])
ifTrue: [self addTextAnchorMenuItems: aMenu hand: evt hand]].
""
self isWorldMorph
ifFalse: [""
aMenu addLine.
self addToggleItemsToHaloMenu: aMenu].
""
aMenu addLine.
self isWorldMorph
ifFalse: [aMenu add: 'copy to paste buffer' translated action: #copyToPasteBuffer:].
+ (Generator on: [:gen | self streamAllStringsAfter: nil on: gen]) in: [:gen |
+ "optimized!! #allStringsAfter: can be slow for large subtrees."
+ gen atEnd ifFalse: [
+ aMenu add: 'copy text' translated action: #clipText]].
- (self allStringsAfter: nil) isEmpty
- ifFalse: [aMenu add: 'copy text' translated action: #clipText].
""
self addExportMenuItems: aMenu hand: evt hand.
""
(Preferences noviceMode not
and: [self isWorldMorph not])
ifTrue: [""
aMenu addLine.
aMenu add: 'adhere to edge...' translated action: #adhereToEdge].
""
self addCustomMenuItems: aMenu hand: evt hand!

Item was changed:
----- Method: Morph>>allStringsAfter: (in category 'debug and other') -----
+ allStringsAfter: aSubmorph
- allStringsAfter: aSubmorph
- "return an OrderedCollection of strings of text in my submorphs. If aSubmorph is non-nil, begin with that container."

+ ^ OrderedCollection streamContents: [:stream |
+ self streamAllStringsAfter: aSubmorph on: stream]!
- | list ok |
- list := OrderedCollection new.
- ok := aSubmorph isNil.
- self allMorphsDo:
- [:sub | | string |
- ok ifFalse: [ok := sub == aSubmorph]. "and do this one too"
- ok
- ifTrue:
- [(string := sub userString) ifNotNil:
- [string isString ifTrue: [list add: string] ifFalse: [list addAll: string]]]].
- ^list!

Item was added:
+ ----- Method: Morph>>streamAllStringsAfter:on: (in category 'debug and other') -----
+ streamAllStringsAfter: aSubmorph on: aStream
+ "Stream all strings of text in my submorphs on aStream. If aSubmorph is non-nil, begin with that container."
+
+ | ok |
+ ok := aSubmorph isNil.
+ self allMorphsDo: [:sub | | string |
+ ok ifFalse: [ok := sub == aSubmorph].
+ "and do this one too"
+ ok ifTrue: [
+ (string := sub userString)
+ ifNotNil: [string isString
+ ifTrue: [aStream nextPut: string]
+ ifFalse: [aStream nextPutAll: string]]]].!




Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Morphic-ct.1771.mcz

Christoph Thiede
Hi Marcel,

yes, I also have thought about this pattern, but I actually don't see why it would be better or why Generator would be an anti-pattern. :-) Your approach requires one more selector than my approach. In general, I think that Generators are a promising way to objectify a lazily/partially evaluatable collection.

Best,
Christoph

> Hi Christoph.
>
> Thanks. I think that the expected pattern is a little bit different, though. The explicit use of Generator is unfortunate but can be easily avoided. :-)
>
> allStringsAfter: morph do: block
>    "..."
>
> allStringsAfter: morph
>    ^ Array streamContents: [:stream |
>       self allStringsAfter: morph do: [:string |
>          stream nextPut: string]]
>
> hasStringsAfter: morph
>    self allStringsAfter: morph do: [:string | ^ true].
>    ^ false
>
> Best,
> Marcel
> Am 25.05.2021 19:39:07 schrieb commits at source.squeak.org <commits at source.squeak.org>:
> A new version of Morphic was added to project The Inbox:
> http://source.squeak.org/inbox/Morphic-ct.1771.mcz
>
> ==================== Summary ====================
>
> Name: Morphic-ct.1771
> Author: ct
> Time: 25 May 2021, 7:38:48.072683 pm
> UUID: 5a9ddaf0-a062-7047-b5b2-d2ae2da3fe15
> Ancestors: Morphic-mt.1769
>
> Fixes a bottleneck when opening a yellow button menu on a morph that contains a very large number of subsub*morphs. On not-so-fast systems, this can be reproduced using:
>
> self systemNavigation browseAllSelect: #notNil
>
> On faster systems, you might need to write a small toolbuilder application to reproduce the bottleneck. I have an app with >10k list items in my image which actually blocked the image for several seconds when I yellow-clicked the window.
>
> Fixed the problem without duplicating the logic of #allStringsAfter: by using a generator.
>
> =============== Diff against Morphic-mt.1769 ===============
>
> Item was changed:
> ----- Method: Morph>>addYellowButtonMenuItemsTo:event: (in category 'menu') -----
> addYellowButtonMenuItemsTo: aMenu event: evt
> "Populate aMenu with appropriate menu items for a
> yellow-button (context menu) click."
> aMenu defaultTarget: self.
> ""
> Preferences noviceMode
> ifFalse: [aMenu addStayUpItem].
> ""
> self addModelYellowButtonItemsTo: aMenu event: evt.
> ""
> Preferences generalizedYellowButtonMenu
> ifFalse: [^ self].
> ""
> aMenu addLine.
> aMenu add: 'inspect' translated action: #inspect.
> ""
> aMenu addLine.
> self world selectedObject == self
> ifTrue: [aMenu add: 'deselect' translated action: #removeHalo]
> ifFalse: [aMenu add: 'select' translated action: #addHalo].
> ""
> (self isWorldMorph
> or: [self mustBeBackmost
> or: [self wantsToBeTopmost]])
> ifFalse: [""
> aMenu addLine.
> aMenu add: 'send to back' translated action: #goBehind.
> aMenu add: 'bring to front' translated action: #comeToFront.
> self addEmbeddingMenuItemsTo: aMenu hand: evt hand].
> ""
> self isWorldMorph
> ifFalse: [""
> Smalltalk
> at: #NCAAConnectorMorph
> ifPresent: [:connectorClass |
> aMenu addLine.
> aMenu add: 'connect to' translated action: #startWiring.
> aMenu addLine].
> ""
>
> self isFullOnScreen
> ifFalse: [aMenu add: 'move onscreen' translated action: #goHome]].
> ""
> Preferences noviceMode
> ifFalse: [""
> self addLayoutMenuItems: aMenu hand: evt hand.
> (owner notNil
> and: [owner isTextMorph])
> ifTrue: [self addTextAnchorMenuItems: aMenu hand: evt hand]].
> ""
> self isWorldMorph
> ifFalse: [""
> aMenu addLine.
> self addToggleItemsToHaloMenu: aMenu].
> ""
> aMenu addLine.
> self isWorldMorph
> ifFalse: [aMenu add: 'copy to paste buffer' translated action: #copyToPasteBuffer:].
> + (Generator on: [:gen | self streamAllStringsAfter: nil on: gen]) in: [:gen |
> + "optimized!! #allStringsAfter: can be slow for large subtrees."
> + gen atEnd ifFalse: [
> + aMenu add: 'copy text' translated action: #clipText]].
> - (self allStringsAfter: nil) isEmpty
> - ifFalse: [aMenu add: 'copy text' translated action: #clipText].
> ""
> self addExportMenuItems: aMenu hand: evt hand.
> ""
> (Preferences noviceMode not
> and: [self isWorldMorph not])
> ifTrue: [""
> aMenu addLine.
> aMenu add: 'adhere to edge...' translated action: #adhereToEdge].
> ""
> self addCustomMenuItems: aMenu hand: evt hand!
>
> Item was changed:
> ----- Method: Morph>>allStringsAfter: (in category 'debug and other') -----
> + allStringsAfter: aSubmorph
> - allStringsAfter: aSubmorph
> - "return an OrderedCollection of strings of text in my submorphs. If aSubmorph is non-nil, begin with that container."
>
> + ^ OrderedCollection streamContents: [:stream |
> + self streamAllStringsAfter: aSubmorph on: stream]!
> - | list ok |
> - list := OrderedCollection new.
> - ok := aSubmorph isNil.
> - self allMorphsDo:
> - [:sub | | string |
> - ok ifFalse: [ok := sub == aSubmorph]. "and do this one too"
> - ok
> - ifTrue:
> - [(string := sub userString) ifNotNil:
> - [string isString ifTrue: [list add: string] ifFalse: [list addAll: string]]]].
> - ^list!
>
> Item was added:
> + ----- Method: Morph>>streamAllStringsAfter:on: (in category 'debug and other') -----
> + streamAllStringsAfter: aSubmorph on: aStream
> + "Stream all strings of text in my submorphs on aStream. If aSubmorph is non-nil, begin with that container."
> +
> + | ok |
> + ok := aSubmorph isNil.
> + self allMorphsDo: [:sub | | string |
> + ok ifFalse: [ok := sub == aSubmorph].
> + "and do this one too"
> + ok ifTrue: [
> + (string := sub userString)
> + ifNotNil: [string isString
> + ifTrue: [aStream nextPut: string]
> + ifFalse: [aStream nextPutAll: string]]]].!
>
>
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20210526/51b6d613/attachment.html>
>
>

Carpe Squeak!
Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Morphic-ct.1771.mcz

marcel.taeumel
I think that we are better off following the existing patterns to improve readability. The use of Generator is surprising and maybe difficult to understand. Keep it simple and common to the surrounding interface.

Best,
Marcel

From: Thiede, Christoph <[hidden email]>
Sent: Friday, May 28, 2021 7:00:14 PM
To: [hidden email] <[hidden email]>; [hidden email] <[hidden email]>
Subject: Re: The Inbox: Morphic-ct.1771.mcz
 
Hi Marcel,

yes, I also have thought about this pattern, but I actually don't see why it would be better or why Generator would be an anti-pattern. :-) Your approach requires one more selector than my approach. In general, I think that Generators are a promising way to objectify a lazily/partially evaluatable collection.

Best,
Christoph

> Hi Christoph.
>
> Thanks. I think that the expected pattern is a little bit different, though. The explicit use of Generator is unfortunate but can be easily avoided. :-)
>
> allStringsAfter: morph do: block
>    "..."
>
> allStringsAfter: morph
>    ^ Array streamContents: [:stream |
>       self allStringsAfter: morph do: [:string |
>          stream nextPut: string]]
>
> hasStringsAfter: morph
>    self allStringsAfter: morph do: [:string | ^ true].
>    ^ false
>
> Best,
> Marcel
> Am 25.05.2021 19:39:07 schrieb commits at source.squeak.org <commits at source.squeak.org>:
> A new version of Morphic was added to project The Inbox:
> http://source.squeak.org/inbox/Morphic-ct.1771.mcz
>
> ==================== Summary ====================
>
> Name: Morphic-ct.1771
> Author: ct
> Time: 25 May 2021, 7:38:48.072683 pm
> UUID: 5a9ddaf0-a062-7047-b5b2-d2ae2da3fe15
> Ancestors: Morphic-mt.1769
>
> Fixes a bottleneck when opening a yellow button menu on a morph that contains a very large number of subsub*morphs. On not-so-fast systems, this can be reproduced using:
>
> self systemNavigation browseAllSelect: #notNil
>
> On faster systems, you might need to write a small toolbuilder application to reproduce the bottleneck. I have an app with >10k list items in my image which actually blocked the image for several seconds when I yellow-clicked the window.
>
> Fixed the problem without duplicating the logic of #allStringsAfter: by using a generator.
>
> =============== Diff against Morphic-mt.1769 ===============
>
> Item was changed:
> ----- Method: Morph>>addYellowButtonMenuItemsTo:event: (in category 'menu') -----
> addYellowButtonMenuItemsTo: aMenu event: evt
> "Populate aMenu with appropriate menu items for a
> yellow-button (context menu) click."
> aMenu defaultTarget: self.
> ""
> Preferences noviceMode
> ifFalse: [aMenu addStayUpItem].
> ""
> self addModelYellowButtonItemsTo: aMenu event: evt.
> ""
> Preferences generalizedYellowButtonMenu
> ifFalse: [^ self].
> ""
> aMenu addLine.
> aMenu add: 'inspect' translated action: #inspect.
> ""
> aMenu addLine.
> self world selectedObject == self
> ifTrue: [aMenu add: 'deselect' translated action: #removeHalo]
> ifFalse: [aMenu add: 'select' translated action: #addHalo].
> ""
> (self isWorldMorph
> or: [self mustBeBackmost
> or: [self wantsToBeTopmost]])
> ifFalse: [""
> aMenu addLine.
> aMenu add: 'send to back' translated action: #goBehind.
> aMenu add: 'bring to front' translated action: #comeToFront.
> self addEmbeddingMenuItemsTo: aMenu hand: evt hand].
> ""
> self isWorldMorph
> ifFalse: [""
> Smalltalk
> at: #NCAAConnectorMorph
> ifPresent: [:connectorClass |
> aMenu addLine.
> aMenu add: 'connect to' translated action: #startWiring.
> aMenu addLine].
> ""
>
> self isFullOnScreen
> ifFalse: [aMenu add: 'move onscreen' translated action: #goHome]].
> ""
> Preferences noviceMode
> ifFalse: [""
> self addLayoutMenuItems: aMenu hand: evt hand.
> (owner notNil
> and: [owner isTextMorph])
> ifTrue: [self addTextAnchorMenuItems: aMenu hand: evt hand]].
> ""
> self isWorldMorph
> ifFalse: [""
> aMenu addLine.
> self addToggleItemsToHaloMenu: aMenu].
> ""
> aMenu addLine.
> self isWorldMorph
> ifFalse: [aMenu add: 'copy to paste buffer' translated action: #copyToPasteBuffer:].
> + (Generator on: [:gen | self streamAllStringsAfter: nil on: gen]) in: [:gen |
> + "optimized!! #allStringsAfter: can be slow for large subtrees."
> + gen atEnd ifFalse: [
> + aMenu add: 'copy text' translated action: #clipText]].
> - (self allStringsAfter: nil) isEmpty
> - ifFalse: [aMenu add: 'copy text' translated action: #clipText].
> ""
> self addExportMenuItems: aMenu hand: evt hand.
> ""
> (Preferences noviceMode not
> and: [self isWorldMorph not])
> ifTrue: [""
> aMenu addLine.
> aMenu add: 'adhere to edge...' translated action: #adhereToEdge].
> ""
> self addCustomMenuItems: aMenu hand: evt hand!
>
> Item was changed:
> ----- Method: Morph>>allStringsAfter: (in category 'debug and other') -----
> + allStringsAfter: aSubmorph
> - allStringsAfter: aSubmorph
> - "return an OrderedCollection of strings of text in my submorphs. If aSubmorph is non-nil, begin with that container."
>
> + ^ OrderedCollection streamContents: [:stream |
> + self streamAllStringsAfter: aSubmorph on: stream]!
> - | list ok |
> - list := OrderedCollection new.
> - ok := aSubmorph isNil.
> - self allMorphsDo:
> - [:sub | | string |
> - ok ifFalse: [ok := sub == aSubmorph]. "and do this one too"
> - ok
> - ifTrue:
> - [(string := sub userString) ifNotNil:
> - [string isString ifTrue: [list add: string] ifFalse: [list addAll: string]]]].
> - ^list!
>
> Item was added:
> + ----- Method: Morph>>streamAllStringsAfter:on: (in category 'debug and other') -----
> + streamAllStringsAfter: aSubmorph on: aStream
> + "Stream all strings of text in my submorphs on aStream. If aSubmorph is non-nil, begin with that container."
> +
> + | ok |
> + ok := aSubmorph isNil.
> + self allMorphsDo: [:sub | | string |
> + ok ifFalse: [ok := sub == aSubmorph].
> + "and do this one too"
> + ok ifTrue: [
> + (string := sub userString)
> + ifNotNil: [string isString
> + ifTrue: [aStream nextPut: string]
> + ifFalse: [aStream nextPutAll: string]]]].!
>
>
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20210526/51b6d613/attachment.html>
>
>