Modules & Signals

dotdotcommadot's Avatar

dotdotcommadot

18 Apr, 2013 05:08 PM

I extended my application ModularRL.
(it was already added to the RL examples as well)

This application started as a demo on how to use RL2 in combination with modules, but since I found some memory leaks coming from the Flex SDK, I decided to take it to a next step and implement services, models etc so I can now check what gets and what doesn't get GC'ed.

I also added signals to the project, to see if there would be any issues comming from there.

Props to RL2 and As3Signals, since I didn't find any leaks comming from you :)

This demo can probably act as a good starting point for people wanting to learn more about

  • MVCS in RL2

  • Signals in RL2

  • modules in RL2

  • modules in Flex, in general

  1. Support Staff 1 Posted by creynders on 18 Apr, 2013 07:31 PM

    creynders's Avatar

    Hi compatriot :)

    I'm curious about the memory leaks you encountered, was this with the latest Flex SDK? Core stuff? Or the horribly written components?

    Just a heads-up: we're completely rewriting the command(maps)-stuff in RL, at a first glance I don't think it will impact your example app, since we try to change the API as little as possible, except where it's an improvement obviously or as an added functionality. Also, I'm rewriting the signal command map completely, but again I don't think it will have a major impact. There were a number of problems with the SCM though, which are getting fixed, those could affect side-effects you've come accustomed to or maybe even count on in your example. Just an FYI, we hope to release v2 soon (don't know when, but soon) and it would be really great to have some examples that can be immediately presented as being definitely up-to-date.

    As for the example, it looks good. Haven't had the time to really tinker with it, but from a not-superficial-but-not-entirely-thorough reading I'd say it'll be very helpful!

  2. Support Staff 2 Posted by Shaun Smith on 19 Apr, 2013 12:11 AM

    Shaun Smith's Avatar

    Heya. Looking good. BTW, is this discussion supposed to be private? (totally fine if it is, it's just that people sometimes make discussions private by mistake)

    Also, I'm curious as to why you're setting the contextView like this:

    https://github.com/dotdotcommadot/ModularRL/blob/master/src/Modular...

    That was a workaround for RL1 where there was no ViewManager to handle multiple display lists per context. Doing that in RL2 negates much of the nested context view optimisation work we've done and isn't nicely scoped.

    Give me a shout if you need some pointers to switch it over. I'm on holiday at the moment, and not always online, but I'll get back to you eventually :)

  3. 3 Posted by dotdotcommadot on 19 Apr, 2013 10:34 AM

    dotdotcommadot's Avatar

    Hi Guys,

    Thank you for the quick reply!
    About the discussion: I put it on private by accident indeed, but I fixed it :)

    I've just put a running application online, published as a:

    Doing this, i noticed that there is still a memory leak, but it's more detailed then I thought it was before.
    Apparently, right now the application only has a severe memory leak when it's running in debug mode in the standalone player.
    Only a part of the used memory will be released after unloading a module, and everytime the app loads the module again, more memory will be used.
    You can test it yourself easily if you:

    • download the debug version of the app here
    • unzip the folder
    • open ModularRL.swf in the standalone flash player
    • click 'Load Module One'
    • click 'Unload Module'
    • load, unload, load, unload, load, unload, .... and watch the memory growing.

    Do the same thing for the release version, which can be found here.
    You will see that here a certain amount of memory will be used, but after 3 times loading/unloading, the memory won't get bigger.

    As long as this only happens in the standalone player in debug mode, I'm actually ok with this.

    About the commands and commandmap rewrite:
    I'll keep an eye out on the forum!
    Once you guys are finished with this, I'll update this example to the latest SDK.
    Thanks for the tip Camille!

    @Shaun, about the contextView: I still use this so my popups get mapped once they land on the stage.
    It works perfectly, but if you think it's a bad practice I will update this.
    In fact, this app currently doesn't use any popups, so I'll just throw it out anyways.
    There are enough topics on the forum on how to implement popups in RL2, so I'll probably figure it out.

    Ps1: I like making demos,I'm open for tips but don't think of myself as a genius, so if you see things in my code that can be optimized according to you guys in any way, I'm more than glad to hear them :)
    Ps 2: I'm thinking about rewriting this app in MVP and MVVM as well, since I tend to use these patterns more often than MVC.

    grts,
    Hans

  4. Support Staff 4 Posted by creynders on 20 Apr, 2013 06:41 AM

    creynders's Avatar

    Hi, I had the time to take a more thorough look.

    It's been a while since I worked with modules in RL1 and I haven't used them in v2 yet, so all of the following comments and suggestions could be completely irrelevant/incorrect/nonsensical:

    1. Why do you include the MVCSBundle in both submodules? Doesn't it defeat the purpose a bit?
    2. Why do you map a stub mediator to the Shell?
    3. One of the things most people struggle with is intermodular communication, maybe a third, persistent module which communicates with the other 2 whenever they're loaded could provide a nice example of that? Or is it out of the scope of your example?
  5. Support Staff 5 Posted by creynders on 20 Apr, 2013 07:00 AM

    creynders's Avatar

    And thanks BTW, by reviewing your example I realized there's a (potential) problem with the SwiftSuspenders Injector, if I'm not mistaken:
    https://github.com/tschneidereit/SwiftSuspenders/issues/88

  6. 6 Posted by dotdotcommadot on 22 Apr, 2013 09:40 AM

    dotdotcommadot's Avatar

    Hi creynders,

    Thanks for looking at the app and your good questions.
    I'll try to answer them as good as I can.

    1. The MVCSBundles in the submodules was because I wanted to see if I could use the Signals extention in a submodule, while the extention wasn't included in the shell. I kind of left this here to keep it "as an example", but you're completely right that this is either irrelevant, or just not clear enough.
      I probably should be more careful with leaving irrelevant things in "as an example", and just either make a new demo for this, or write it in down the comments.
      I will for now just remove it, and keep the app to its essence.
    2. The shellMediator used to trigger a command which loaded and unloaded the module (I used the ModuleManager to do this).
      I refactored this so now the Shell loads and unloads this module with the ModuleLoader (I think it's probably OK to keep this in the view... since it's only view functionality).
      I kept the ShellMediator just for uniformity throughout the application, to make the demo clear, as one of the scopes of this demo is "how to use the MVCS pattern". But then again, you could say this is the same mistake as I made in point 1 hehe. But actually, I don't think so.
    3. Great idea, I'll add this to the example.
  7. Support Staff 7 Posted by creynders on 22 Apr, 2013 10:32 AM

    creynders's Avatar

    I kept the ShellMediator just for uniformity throughout the application, to make the demo clear, as one of the scopes of this demo is "how to use the MVCS pattern".

    Sure, sounds valid to me, maybe you just have to let it "do" something, no matter how nonsensical, so it's clearly there for educational purposes.

    Great idea, I'll add this to the example.

    Yeah, maybe you should wait a moment, I'm going to try and rewrite the intermodular communication stuff, it's pretty ugly at the moment.

  8. 8 Posted by dotdotcommadot on 22 Apr, 2013 04:48 PM

    dotdotcommadot's Avatar

    I just commited a few changes:

    1. I noticed I can't just remove the custom "MVCSBundles". If I do so, my context isn't getting triggered, right?
      I could replace <config:LogModuleBundle /> with <mvcs:MVCSBundle/>, but this seems wrong as well because I need to add the SignalCommandMapExtension to the bundle.
      So I kept them here. Do you think there's a better way?
    2. removed ShellMediator after all. There are enough MVCS examples troughout the rest of the demo.
    3. I added intermodular communication. although it is not a "clean" implementation yet, I think it exposes the current issues with this, and therefore is a good example ofa workaround.
      I mapped the signals to the commands, inside the ShellConfig.
      But I think it is a bit double: you could say the shell is not the right place to map this, because it breaks the encapsulation. (The shell knows about the workings of the modules, so this seems wrong). But on the other hand, you could say that it would be "weird" to map the addLogSignal in ModuleTwoConfig, because ModuleTwo doesn't need to know of the AddLogCommand either.
      I think, if you have a look at ShellConfig, you'll see what I mean..

    As always, comments are appreciated :)

  9. Support Staff 9 Posted by creynders on 23 Apr, 2013 09:16 AM

    creynders's Avatar

    As I wrote somewhere I haven't really looked into modules yet in v2, so I'm afraid I can't really help on what exactly is required to get them to work properly. I hope Shaun has time to drop by and comment.

  10. Support Staff 10 Posted by Ondina D.F. on 23 Apr, 2013 02:45 PM

    Ondina D.F.'s Avatar

    Hey Hans,

    I have just a little time to make some comments regarding your demo. I’m addressing only your gc-issues in this post.
    Your modules don’t get gc-ed because they never leave the stage, and that’s because of the way you’re using skins and css styles.
    The skin classes are keeping a reference to the views, i.e. ModuleOneMainViewSkin to ModuleOneMainView and ModuleTwoMainViewSkin to ModuleTwoMainView, because the css styles don’t get removed when the modules are unloaded.

    A workaround:
    Instead of the fx style declaration, set the style within the constructor of the view like this:

    public function ModuleOneMainView()
    {
    setStyle("skinClass", Class(ModuleOneMainViewSkin));
    }
    

    When the view gets removed from stage, the styleManager will remove the style automatically.

    Even with the above workaround, ModuleOne is never leaving the stage.
    ModuleOneMainViewSkin contains a datagrid, which keeps the view and the skin alive.
    After replacing the datagrid with a spark list, the skin and the view get removed from stage.
    I’m not sure and I don’t have the time to investigate this any further, but I think it is a (datagrid) bug.

    In my opinion, it would be better (and easier) to use simple components for your demo, without styles and skins. Keep the focus on modularity.

    Not an issue in your case, but sometimes having something like this:

    view.loadUsersButton.addEventListener( MouseEvent.CLICK, onClick_loadUsersButton );
    

    in your mediator, can result in gc-issues, if you don’t remove the event listeners.

    A better approach would be:

    addViewListener(SomeEvent.SOMETHING_HAPPENED, onSomethingHappened, SomeEvent);
    

    where SomeEvent.SOMETHING_HAPPENED would be dispatched by the view in the handler of the loadUsersButton when clicked.

    Also not a problem in your case, but I’m just mentioning it:
    Something that can help a lot when dealing with gc, especially in case of modules, is cleaning up views before they are removed from stage: removing all event listeners, removing child components, setting dataproviders to null, etc. In other words, make sure to get rid of any references that could keep your views alive.

    That’s all for now :)
    Ondina

  11. Support Staff 11 Posted by Ondina D.F. on 26 Apr, 2013 02:33 PM

    Ondina D.F.'s Avatar

    Hey Hans,

    I found some time today to take another look at your app. I also re-read the previous posts more attentively and noticed that you added a LogModule. So, I dl-ed the new version and while I was at it, I finally read your README. You should have told me to RTFM ;)
    Indeed, having this compiler argument: -keep-all-type-selectors, solves the issue with unloading a module’s style, and there is no need for setting the skinClass in actionscript anymore. But, the DataGrid issues still remain. Hopefully you’ll have more time than me to investigate that.

    I still think that you should concentrate only on the rl-modular aspect in this demo, keeping it simple -> no styles or skins, no compiler arguments...
    You could make another demo, expanding the simple one, which focuses on memory/gc issues.

    Your comments inside of ShellConfig:

    // Intermodular communication workaround // On one side, you could say that this is actually in the wrong scope: // the shell application shouldn't have to know about these. // on the other side: moduleTwo shouldn't have to know about the addLogCommand either I think, // so where else do you map the signal to the command, instead of in the shell?

    You’re right, it’s tricky.
    Just thinking out loud:
    To me, LogModel, AddLogCommand and AddLogSignal seem to belong to the LogModule instead of being shared, and should be mapped in LogConfig. But, how to dispatch the signal from ModuleTwo in this case?
    Maybe by having an IntermodularSignal (the name is just for the sake of an example) mapped in the ShellConfig, which would serve as a ‘global’ channel?
    Then, dipatch IntermodularSignal from ModuleTwo, or any other module to be listened by LogModule. It would be cool if we had a way to listen to that ‘global’ signal inside the module, and to relay it to a ‘local’ one, i.e. AddLogSignal, which would trigger AddLogCommand.

    See the discussion for a similar problem with events: http://knowledge.robotlegs.org/discussions/robotlegs-2/931-intermod...

    On another note, you certainly know that injecting models into mediators is a disputed practice, don’t you? ;)
    Personally, I’m not strictly against it, sometimes it really makes sense to do so depending on the use case, but since you want your demo to be an ‘official’ one, or so I think, it would be nice if you’d follow the stricter MVCS-way. The suggestion from my previous post regarding addViewListener falls into the same category.

    Hopefully, you won’t take my observations the wrong way. They aren’t meant to be destructive :)

    Ondina

  12. 12 Posted by dotdotcommadot on 26 Apr, 2013 04:17 PM

    dotdotcommadot's Avatar

    Hi Ondina,

    thanks a lot for commenting so thoroughly; most certainly not taken the wrong way!

    • About the skins: The reason why I used them in this demo was to test if it's true that it won't cause memory leaks when using ' -keep-all-type-selectors'. But now that I know the results, I think you're right and I should remove them for clarity of the demo. Or better: I'll change this demo to the "advanced" one, and simplify it for the other demo.

    • You're also right about the shortcuts I took. I'll remove them, and stick to clean MVCS.

    • I've been thinking about the intermodular issue a lot., One other issue is that Signals don't have weak references.
      otherwise you could just make some sort of singleton "GlobaSignalsMap", and listen to all of the signals on that...
      But as far as I got to test, components won't get GC'd when they are still mapped to a signal.

    Anyway,I'll try to fix things asap, but now need to focus on a deadline at the end of next week :)

    Hans

  13. Support Staff 13 Posted by Ondina D.F. on 26 Apr, 2013 04:46 PM

    Ondina D.F.'s Avatar

    thanks a lot for commenting so thoroughly;

    My pleasure!

    most certainly not taken the wrong way!

    Glad to hear that.

    Or better: I'll change this demo to the "advanced" one, and simplify it for the other demo.

    Cool:)

    One other issue is that Signals don't have weak references. otherwise you could just make some sort of singleton "GlobaSignalsMap", and listen to all of the signals on that...
    But as far as I got to test, components won't get GC'd when they are still mapped to a signal.

    That’s also why, as much as I love signals, I use them sparingly and with lots of care in my projects. Over the years I’ve become quite obsessed with gc/memory leaks;)

    Anyway,I'll try to fix things asap, but now need to focus on a deadline at the end of next week :)

    Take your time and good luck with your deadline.

    I’ll close this thread for now. Please re-open it when and if you want to continue the discussion or when you have new versions of your demo.

    Ondina

  14. Ondina D.F. closed this discussion on 26 Apr, 2013 04:46 PM.

Comments are currently closed for this discussion. You can start a new one.

Keyboard shortcuts

Generic

? Show this help
ESC Blurs the current field

Comment Form

r Focus the comment reply box
^ + ↩ Submit the comment

You can use Command ⌘ instead of Control ^ on Mac