- Pharo-dev - lists.pharo.org

Re: [Pharo-dev] Rubric: spotter with preview leaks some memory

by Nicolai Hess

2016-01-25 20:10 GMT+01:00 Nicolai Hess <nicolaihess(a)gmail.com>: > > > 2016-01-20 23:23 GMT+01:00 Nicolai Hess <nicolaihess(a)gmail.com>: > >> >> >> 2016-01-20 19:18 GMT+01:00 Nicolai Hess <nicolaihess(a)gmail.com>: >> >>> >>> >>> 2016-01-20 14:47 GMT+01:00 Andrei Chis <chisvasileandrei(a)gmail.com>: >>> >>>> Tried it in the last latest image and if I execute before >>>> #garbageCollect I just get 4 instances. >>>> >>>> Smalltalk garbageCollect. >>>> RubTextLine allInstances size "4" >>>> >>>> Now indeed if I browse the same class in nautilus I get a much smaller >>>> number of RubTextLine instances >>>> >>> >>> >>> I opened a bugreport >>> 17437 >>> <https://pharo.fogbugz.com/f/cases/17437/Rubric-spotter-with-preview-leaks-s…> >>> >>> Rubric: spotter with preview leaks some memory >>> >> >> >> I tried to reproduce this behavior with GTInspector instead of Spotter, >> for example, inspect: >> >> MethodFinder methods >> select on entry, -> another inspector pane shows up. Select th >> "Source"-tab. Now >> scroll through the list of methods in the first inspector pane. For every >> method the second pane updates the >> source tab with the code of the selected method. >> >> This generates many RubTextLine instances too. But not as many and I can >> always clean them up with Smalltalk garbageCollect. >> So, the is something that spotter does differently. >> > > And even if Smalltalk garbageCollect would clean all instances, > I don't think it is right, that scrolling through the method list (with > preview) should create hundred thousends of RubTextLine instances. > There is just not enough code visible, we definitly don't need that much. > There is something wrong. > Maybe these are all related 15153 <https://pharo.fogbugz.com/f/cases/15153/Rubric-mutability-issue-in-RubTextC…> Rubric mutability issue in RubTextComposer binary search 17284 <https://pharo.fogbugz.com/f/cases/17284/Rubric-SubscriptOutOfBounds-error> Rubric SubscriptOutOfBounds error 17447 <https://pharo.fogbugz.com/f/cases/17447/Playground-and-long-number-crashes-…> Playground and long number crashes with SubscriptOutOfBounds This may be an issue with text composer, called after styling and (I think) concurrent processes access and modifiy the RubTextComposers #lines attribute. some other observations: RubTextComposer>>#needComposition compares prevText / newText (attributes) runs ret := prevText runs ~= theText runs but both runs are often "different" even if they are for the same text: The text link classes are compared by identity instead of equality. And code like this RubShoutStyleDecorator>>replaceFrom: start to: stop with: aText self okToStyle ifFalse: [ ^ next replaceFrom: start to: stop with: aText ]. self disableDrawingWhile: [ aText addAttribute: self defaultFontChange. "<<<<--------- THIS" text ifNil: [ text := self text ]. text replaceFrom: start to: stop with: (self styler format: aText). will *always* add a trailing font change attribute, even if the prior styled and new styled text would be actually the same (originate from the same raw string)! there are multiple calls like aText addAttribute: self defaultFontChange. in other methods. GLMHighlighterTextStylerDecorator looks similiar with some minor changes (for no obvious reason and of course totally undocumented). Debugging this code is a pain with all the doesNotUnderstand redirection Analysing this code is a pain with all the undocumented code, copy and pasted to GLM (GLMHighlighterTextRubEditingMode/GLMHighlighterTextStylerDecorator/GLMHighlighterTextParserStyler....) copy and pasted *from* old text classes (with no indication if this is copied because it is used the same way or just DEAD code). And for real, does anyone work with spotter and preview enabled ? I can not work with it for more than some minutes without triggering this allocation of awfull many RubTextLine instances. I have to manually kill the styling process. no one? > > > >> >> >> >> >>> >>> >>> >>> >>>> >>>> On Wed, Jan 20, 2016 at 2:32 PM, Nicolai Hess <nicolaihess(a)gmail.com> >>>> wrote: >>>> >>>>> Open fresh image 50536 >>>>> Open Spotter, >>>>> enable preview (cmd+p) >>>>> search for EllipseMorph >>>>> dive in instance-methods >>>>> scroll through the list one item after another. >>>>> >>>>> open playground >>>>> evaluate >>>>> RubTextLine allInstances size. "553896" >>>>> >>>>> >>>>> case 17421 is related to this, but does not fully solves this. >>>>> please review and comment on that fix, >>>>> so we can integrate this soon. >>>>> >>>>> The problem sometimes remains even with this fix. The memory >>>>> (instance count of RubTextLine) just grows in the background >>>>> >>>> >>>> >>> >> >

Jan. 31, 2016

Re: [Pharo-dev] Contributing to Pharo

by Ben Coman

On Sun, Jan 31, 2016 at 3:38 AM, Dale Henrichs <dale.henrichs(a)gemtalksystems.com> wrote: > > > On 1/30/16 1:54 AM, Bernhard Pieber wrote: >> >> Dale, >> >> Thanks for your thorough answer. I really appreciate how you include links >> to helpful articles. >> >> I find the description of the workflow you actually use very enlightening. >> However, one thing still remains unclear. In the last step, when merging the >> pull request. How is the unchanged metadata reconciled with the code >> changes? I just realized that I just donât know what information is in the >> Monticello metadata, which is not in the code? >> > Monticello metadata is basically the entire Monticello version history of > the package, it includes direct ancestors, commit comments, the GUID, etc. > For a FileTree repo, the meta data is stashed a separate file ... The form > of the data is actually serialized Smalltalk object graph - a deeply nested > set of Arrays - all written on a single line, so git has very little chance > of being able to merge two files What would it take to have the meta data spread out over multiple lines (if that would work better with git?) cheers -ben

Jan. 31, 2016

17502. Morph layoutProperties must not be nil by default

by Aliaksei Syrel

Hello Do you agree? https://pharo.fogbugz.com/f/cases/17502/Morph-layoutProperties-must-not-be-… Cheers, Alex

Jan. 30, 2016

Re: [Pharo-dev] Contributing to Pharo

by Dale Henrichs

On 1/29/16 2:45 PM, David Allouche wrote: > Thanks Dale for all the explanations. > > How Monticello and version control relate in the big picture is starting to make sense for me. > > Now, I better understand why filetree ended up uses a file-per-method format, even though that is relatively hostile to git user interfaces optimised for other languages. There is really a need for a file-per-class exchange format, because that would works a lot better with the existing VCS ecosystem. I agree 100%, but as I've mentioned in another post, I think that switching to a file-per-class exchange format needs to be a cross-dialect effort. A format that accommodates the diversity of Smalltalk class definition and even method metadata while still being readable is a bit of a challenge - solvable, but challenging.... Personally, I have other fish to fry, but I would be willing to participate in such an effort if someone else is willing to lead the charge:) Dale

Jan. 30, 2016

Re: [Pharo-dev] Contributing to Pharo

by Dale Henrichs

On 1/30/16 1:54 AM, Bernhard Pieber wrote: > Dale, > > Thanks for your thorough answer. I really appreciate how you include links to helpful articles. > > I find the description of the workflow you actually use very enlightening. However, one thing still remains unclear. In the last step, when merging the pull request. How is the unchanged metadata reconciled with the code changes? I just realized that I just donât know what information is in the Monticello metadata, which is not in the code? > Monticello metadata is basically the entire Monticello version history of the package, it includes direct ancestors, commit comments, the GUID, etc. For a FileTree repo, the meta data is stashed a separate file ... The form of the data is actually serialized Smalltalk object graph - a deeply nested set of Arrays - all written on a single line, so git has very little chance of being able to merge two files ... The other bit of Monticello metadata that is maintained by FileTree involves the per method timestamp and developer initials - these are stored in a method properties file associated with each class. In fact is nearly impossible for a human to properly merge to Monticello version histories, which is why Thierry Goubier's GitFileTree-MergeDriver[1] is especially useful. Thierry's MergeDriver fires up a Pharo image (called by git) and performs object surgery to merge the conflicting version histories and method properties. Anyway, in the scenario that I described, I simplified the process a bit and left out some of the gorier details. When I cherry-picked the commit for my topic branch and merged that commit into Sven's latest master branch, the Monticello metadata was certain to conflict, but when you perform a merge on a local git repository and you've registered Thierry's merge driver with git, the Monticello metadata is automatically merged. I also made the assumption that Sven would not have made any additional commits to his master branch between the time I merged his master branch and the time I submitted the pull request - the latest version of a file wins in a merge as long as they share the same immediate previous ancestor. If Sven had made a new commit, the Monticello metadata would have conflicted ... For the record, these "guaranteed conflicts" are the only reason that "removing Monticello metadata" enters into the conversation .... git maintains it's own version history and method properties, so for developers who are "exclusively using git" the Monticello metadata is an annoying source of "meaningless conflicts." All of the information that is present in Monticello meta data is present in git in another form As I said, Thierry Goubier's magical GitFileTree-MergeDriver[1] eliminates the conflict pain when the merges are performed in a local git repository, but on GitHub, merging the Monticello metadata is a constant thorn in one's side, since it becomes necessary to perform a manual merge (and leverage Thierry's Merge Driver) when Monticello metadata is involved. Dale [1] https://github.com/ThierryGoubier/GitFileTree-MergeDriver

Jan. 30, 2016

Re: [Pharo-dev] Disable assertions (design by contract)

by stepharo

Nicolai some people use assert: (defined in Object) as precondition checker. Stef

Jan. 30, 2016

[pharo-project/pharo-core]

by GitHub

Branch: refs/tags/50560 Home: https://github.com/pharo-project/pharo-core

Jan. 30, 2016

[pharo-project/pharo-core] 2fb151: 50560

by GitHub

Branch: refs/heads/5.0 Home: https://github.com/pharo-project/pharo-core Commit: 2fb151c3eea6bf2b829178b41fd7b7acb3b664f3 https://github.com/pharo-project/pharo-core/commit/2fb151c3eea6bf2b829178b4… Author: Jenkins Build Server <board(a)pharo-project.org> Date: 2016-01-30 (Sat, 30 Jan 2016) Changed paths: A Deprecated50.package/extension/SmalltalkImage/instance/addDeferredStartupAction_.st A Deprecated50.package/extension/SmalltalkImage/instance/addToShutDownList_.st A Deprecated50.package/extension/SmalltalkImage/instance/addToShutDownList_after_.st A Deprecated50.package/extension/SmalltalkImage/instance/addToShutDownList_before_.st A Deprecated50.package/extension/SmalltalkImage/instance/addToStartUpList_.st A Deprecated50.package/extension/SmalltalkImage/instance/addToStartUpList_after_.st A Deprecated50.package/extension/SmalltalkImage/instance/addToStartUpList_before_.st A Deprecated50.package/extension/SmalltalkImage/instance/add_toList_after_.st A Deprecated50.package/extension/SmalltalkImage/instance/add_toList_before_.st A Deprecated50.package/extension/SmalltalkImage/instance/removeFromShutDownList_.st A Deprecated50.package/extension/SmalltalkImage/instance/removeFromStartUpList_.st R ScriptLoader50.package/ScriptLoader.class/instance/pharo - scripts/script50559.st A ScriptLoader50.package/ScriptLoader.class/instance/pharo - scripts/script50560.st R ScriptLoader50.package/ScriptLoader.class/instance/pharo - updates/update50559.st A ScriptLoader50.package/ScriptLoader.class/instance/pharo - updates/update50560.st M ScriptLoader50.package/ScriptLoader.class/instance/public/commentForCurrentUpdate.st M System-BasicCommandLineHandler.package/BasicCommandLineHandler.class/class/system startup/startUp_.st M System-Support.package/SmalltalkImage.class/instance/accessing/session.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/changeImagePathTo_.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/changesFile.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/changesName.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/changesSuffix.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/fileForChangesNamed_.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/fileForImageNamed_.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/fullNameForChangesNamed_.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/fullNameForImageNamed_.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/imageDirectory.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/imageFile.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/imagePath.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/imagePath_.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/imageSuffix.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/shortImageName.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/sourceFileVersionString.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/sourcesFile.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/sourcesName.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/sourcesSuffix.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/vmBinary.st A System-Support.package/SmalltalkImage.class/instance/image%2C changes name/vmDirectory.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/changeImagePathTo_.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/changesFile.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/changesName.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/changesSuffix.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/fileForChangesNamed_.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/fileForImageNamed_.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/fullNameForChangesNamed_.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/fullNameForImageNamed_.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/imageDirectory.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/imageFile.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/imagePath.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/imagePath_.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/imageSuffix.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/shortImageName.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/sourceFileVersionString.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/sourcesFile.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/sourcesName.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/sourcesSuffix.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/vmBinary.st R System-Support.package/SmalltalkImage.class/instance/image%2C changes names/vmDirectory.st R System-Support.package/SmalltalkImage.class/instance/snapshot and quit/addDeferredStartupAction_.st M System-Support.package/SmalltalkImage.class/instance/snapshot and quit/snapshot_andQuit_.st R System-Support.package/SmalltalkImage.class/instance/startup list/addToShutDownList_.st R System-Support.package/SmalltalkImage.class/instance/startup list/addToShutDownList_after_.st R System-Support.package/SmalltalkImage.class/instance/startup list/addToShutDownList_before_.st R System-Support.package/SmalltalkImage.class/instance/startup list/addToStartUpList_.st R System-Support.package/SmalltalkImage.class/instance/startup list/addToStartUpList_after_.st R System-Support.package/SmalltalkImage.class/instance/startup list/addToStartUpList_before_.st R System-Support.package/SmalltalkImage.class/instance/startup list/add_toList_after_.st R System-Support.package/SmalltalkImage.class/instance/startup list/add_toList_before_.st R System-Support.package/SmalltalkImage.class/instance/startup list/removeFromShutDownList_.st R System-Support.package/SmalltalkImage.class/instance/startup list/removeFromStartUpList_.st M Zinc-HTTP.package/ZnServer.class/class/system startup/startUp_.st Log Message: ----------- 50560 16735 Replace Startup/Shutdown list with SessionManager https://pharo.fogbugz.com/f/cases/16735 http://files.pharo.org/image/50/50560.zip

Jan. 30, 2016

Re: [Pharo-dev] ProcessTest>>testHighPriorityOverridesWaitTime intermittent failures

by Denis Kudriashov

Hi. I like your proposals. It fixed problem. And more important it provides better documentation for tested behaviour 2016-01-30 10:43 GMT+01:00 Ben Coman <btc(a)openinworld.com>: > Case 17373 had an erroneous failure from > ProcessTest>>testHighPriorityOverridesWaitTime > and several others seem also to have suffered... > * https://pharo.fogbugz.com/default.asp?17373 > * https://pharo.fogbugz.com/default.asp?17496 > * https://pharo.fogbugz.com/default.asp?17445 > * https://pharo.fogbugz.com/default.asp?7444 > > These have comments that the problem can't be reproduced, which is > common of occasional race conditions. So rather than just try to > solve this on the issue tracker, I thought I'd share a tip how to > isolate and squash such intermittent failures, essentially by > stressing that single test a large number of times like this... > > 1 to: 100 do: [ :n | Transcript crShow: 'Try ' , n printString. > ProcessTest debug: #testHighPriorityOverridesWaitTime. ] > > Since the debugger only focusses on a point in time snapshot of the > running system, some Transcript instrumentation was useful to overview > the dynamic behaviour, like this... > > ProcessTest>>testHighPriorityOverridesWaitTime > | lowerPriorityWaitingLonger higherPriorityWaitingLess > nextReadyProcess | > lowerPriorityWaitingLonger := [ ] forkAt: 11 > higherPriorityWaitingLess := [ ] forkAt: 12. > nextReadyProcess := Processor nextReadyProcess. > Transcript cr; > tab;show: (lowerPriorityWaitingLonger priority -> > lowerPriorityWaitingLonger identityHash); cr; > tab;show: (higherPriorityWaitingLess priority -> > higherPriorityWaitingLess identityHash); cr; > tab;show: (nextReadyProcess priority -> nextReadyProcess > identityHash). > self assert: nextReadyProcess equals: higherPriorityWaitingLess. > > which produces... > Try 1 > 11->343137024 > 12->40659200 > 12->40659200 > Try 2 > 11->461794304 > 12->374996992 > 12->40659200 > > Observe on the second try nextReadyProcess refers to same process as > from the first try. This can be cleaned up just prior to the asset > check by adding ... > lowerPriorityWaitingLonger terminate. > higherPriorityWaitingLess terminate. > which produces... > Try 1 > 11->1005688064 > 12->833273600 > 12->833273600 > ... > Try 100 > 11->764450816 > 12->807973888 > 12->807973888 > > However this is still a little fragile, since some process with > priority greater than the running test could slip in before the call > to nextReadyProcess, as can be seen by pushing the #to:do: loop out to > 100000, which produces... > Try 32044 > 11->784361216 > 12->431928576 > 40->397533952 > > or wrapping it by [ ]forkAt:39, which produces... > Try 1 > 11->431897088 > 12->400972288 > 30->338363648 > > After several awkward failed attempts to fix this, it seemed best to > observe actual behaviour rather than peeking into the dynamic data > structures. Thus I propose... > > ProcessTest>>testHighPriorityOverridesWaitTime > "The first process to resume will pass straight through the gate > while the other waits for the assert to whichRan." > > | gate checkAssert priorityRan | > gate := Semaphore new signal. > checkAssert := Semaphore new. > > [ gate wait. priorityRan := 11. checkAssert signal ] forkAt: 11. > [ gate wait. priorityRan := 12. checkAssert signal ] forkAt: 12. > > checkAssert wait. > self assert: priorityRan=12 description: 'Second scheduled but > higher priority should run first'. > gate signal. > > checkAssert wait. > self assert: whichRan=11 description: 'First scheduled but lower > priority should run after'. > > https://pharo.fogbugz.com/default.asp?17501 > >

Jan. 30, 2016

Re: [Pharo-dev] Contributing to Pharo

by Bernhard Pieber

Hi David, Just for your information, Cuis uses a file-per-package format. It looks really nice on GitHub and on SourceTree. As far as I can tell, there has not been much merging going, though. So it is entirely possible that it turns out not to work as well as the filetree format when merging. Cheers, Bernhard > Am 29.01.2016 um 23:45 schrieb David Allouche <david(a)allouche.net>: > > Thanks Dale for all the explanations. > > How Monticello and version control relate in the big picture is starting to make sense for me. > > Now, I better understand why filetree ended up uses a file-per-method format, even though that is relatively hostile to git user interfaces optimised for other languages. There is really a need for a file-per-class exchange format, because that would works a lot better with the existing VCS ecosystem. > > I think more package-based user interfaces would indeed be a very good idea, for browsing and for source code management. > > Stef, I have the impression you think that git is popular because it is a new shiny toy. I disagree with this idea. Git is a typical worse-is-better tool. It's good enough for most people, but it still has many shortcomings. It is popular in spite of its shortcomings. It became popular as destination for projects shifting from CVS and Subversion. So it is unlikely to be displaced by a newer, incrementally shinier tools. Anything that will displace it will have to provide an improvement of a similar magnitude as the jump between centralised and distributed version control. > > Still, I think it's a good idea not to restrict high level models to what git provides if that's a less than ideal fit to the image model. > > I have a lot of ideas to improve browsing and source code management in Pharo. I can make no promises, but I would like to produce something there.

Jan. 30, 2016