I have just committed a fix for this issue, based on the "alternative option": - removed both StageText => ScrollableStageText and ScrollableStageText => StageText maps from StageTextPool - SST maintains a reference to its latest StageText , so the maps are not needed anymore - added some sanity checks so that savedStageText cannot be used if it has been disposed by the cyclical purge, or reused by another TextInput.
I did two tests with both persistent TI container (same instance added/removed), and non-persistent. In both cases, GC is done properly (in the profiler) CheckinTests and Mustella Mobile/TextInput test pass. So it's looking good to me. Can someone please review the changes, in case I missed something: Diff: http://git-wip-us.apache.org/repos/asf/flex-sdk/diff/36cece0d Maurice -----Message d'origine----- De : Maurice Amsellem [mailto:maurice.amsel...@systar.com] Envoyé : mercredi 23 avril 2014 02:12 À : dev@flex.apache.org Objet : RE: Question about mobile StageText pool > I'm surprised that each skin instance doesn't have its own > ScrollableStageText. I would think only the StageText instances are pooled. Actually, this is the case: only StageText are cached. SST acts as a *key* in the reverse dictionary (SST => ST). Stage text is released (and put into the cache) when SST is removed from the stage. If the same SST is added back to the stage, and it's ST is still in the cache, then it gets the same ST instance. That's why a map SST to ST is needed. > I think you also have the option of making the back referencing pool a weak > reference dictionary. It's already using weak keys: private var map_StyleableStageText_to_StageText:Dictionary = new Dictionary(true); private var map_StageText_to_StyleableStageText:Dictionary = new Dictionary(true); It's worse than what I thought: I replaced all the event listeners to use weak references, still does not work. This is because each SST instance that is used as a key in the pool reverse map is still referencing: - TextInputSkin (in styleName, parent, owner, automationParent, automationOwner) - TitleWindowSkin (in document, parentDocument)... - probably other referenced I didn't see ... So it's locking them (TextInputSkin and TitleWindowSkin) in memory. I tried nulling the references when the SST is removed from stage, does not work. I also tried to disable the pool, => the instances are correctly released, so at least we know where the problem is, but this is not an option. I thought about another possibility would be to use the SST itself as a map, instead of a static map (SST => ST) . This could be done as follows: - each SST will have a "savedStageText" variable, which contains the last StageText instance, when the SST is not on the stage - whenever the SST is put back to stage, if it has a 'savedStageText', it will be used instead of allocating a new SST. - new SST will get a StageText from the pool of unused stage texts (as currently) - we also need a mechanism to avoid having too many savedStageText instances (which would overflow the OS memory). Maybe something like a counter I will sleep on it ... Maurice -----Message d'origine----- De : Alex Harui [mailto:aha...@adobe.com] Envoyé : mardi 22 avril 2014 19:58 À : dev@flex.apache.org Objet : Re: Question about mobile StageText pool I think I understand your description, but I'm surprised that each skin instance doesn't have its own ScrollableStageText. I would think only the StageText instances are pooled. It seems ok to use removeFromStage to cut any references between the StageText and the ScrollableStageText since a Skin not on the display list has no need for a StageText. I think you also have the option of making the back referencing pool a weak reference dictionary. -Alex On 4/22/14 10:31 AM, "Maurice Amsellem" <maurice.amsel...@systar.com> wrote: >>When I look at SkinnableTextBase.partAdded it looks like it is adding >>a listener to the 'textDisplay'. I assume that 'textDisplay' isn't a >>StageText in a pool. If that's true, >the SkinnableTextBase.as is >>correct. > >I would expect that 'textDisplay' is a StageTextInputSkin and >internally it should be adding weak reference listeners to the actual >StageText's in the pool. Or are those >bad assumptions? > >It's a little trickier than that, because StageText itself is wrapped >in a ScrollableStageText (or StyleableStageText depending on the skin) > >So SkinnableTextBase.textDisplay is the ScrollableStageText which is a >(pooled) wrapper around StageText. > >And the pool has two static dictionaries ( SST => ST and ST => SST). > >So seeting the listeners on the SST locks the TI, because the SST are >also referenced in the pool dictionary. > >Makes sense to you? > >Maurice > >-----Message d'origine----- >De : Alex Harui [mailto:aha...@adobe.com] Envoyé : mardi 22 avril 2014 >19:20 À : dev@flex.apache.org Objet : Re: Question about mobile >StageText pool > > > >On 4/22/14 10:07 AM, "Maurice Amsellem" <maurice.amsel...@systar.com> >wrote: > >>>so detaching skins does not have to be part of the lifecycle. >>I agree with that, that's why I was asking about removing listeners, >>rather than detaching skins. Is that the same ? >>IOW, do you mean that explicitly removing listeners from the skin to >>the component shouldn't be part of the component lifecycle, and all >>rely on GC ? >Either way, there is no good event/trigger to use to know when to >remove listeners or detach skins so I would not make it part of the lifecycle. > >> >>> Isn't the solution as simple as using weak reference listeners to >>>the stagetext events? >>Yes, it's probably that simple ( I have to check yet). >>But the events are not set in the skins, they are set in the component >>(SkinnableTextBase.partAdded / partRemoved). >>So doing it that way bothers me because the component is not supposed >>to know about the internals of the skins (pooling , or whatever). >>So setting weak listeners in the component because we KNOW that the >>skin is using a pool defeats that principle. >> >>But maybe I am too "purist" ;-) >When I look at SkinnableTextBase.partAdded it looks like it is adding a >listener to the 'textDisplay'. I assume that 'textDisplay' isn't a >StageText in a pool. If that's true, the SkinnableTextBase.as is correct. > I would expect that 'textDisplay' is a StageTextInputSkin and >internally it should be adding weak reference listeners to the actual >StageText's in the pool. Or are those bad assumptions? > >-Alex >> >
