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]]]].! |
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
|
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!
|
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> > > |
Free forum by Nabble | Edit this page |