1. Welcome to LilyPad. Download the project, explore the forums, and create your own LilyPad network.

    If you use the software and enjoy it or have a question, or would like to contribute to the future of the software directly or through resources, please sign up and join our little community.

Connect API Listening for Events

Discussion in 'Developer Ramblings' started by Coelho, Dec 15, 2013.

  1. Coelho

    Coelho Software Engineer Staff Member Administrator Maintainer

    As of the moment, the Connect server is relaying all events (Server, Message, and Redirect) to all servers regardless of whether they listen for it or not. This causes unnecessary bandwidth usage.

    We should remedy this issue with a few steps:
    1. Connections must send "registerEvent" and "unregisterEvent" packets to the Connect Server before they receive an event. This would be entirely transparent within the APIs.
    2. We need to implement filters, where a MessageEvent will not be handled unless we are listening the certain channel we expect.
    Issues with the 1st step:
    1. none?
    Issues with the 2nd step:
    1. The filters have to work with all events. I would say the best way to do this would be something like {channel: "test"} or something of the sort. Implementing this in Java we'd have to make a util class for it so you could do something like this: new Filter("channel", "test").and("message", new byte[0]);
    2. Current plugins which do not have filters will need to register all channels regardless of filter. This kind of breaks the optimization, but at the same time it is just undoing this change.
    1. In JLilyPad we should rename @EventListener to @ConnectEventHandler, but leave backwards compatibility for older plugins. I believe the initial was a poor design choice.
    2. Is it time to remove our deprecated methods?
    Please leave your input. Thanks :)
  2. boboman13

    boboman13 Member Contributor

    @Coelho, would we see any excessive bandwidth usage with the registerEvent and unregisterEvent if they're used incorrectly by plugins? If a programmer doesn't really use it correctly, they could be registering channels for lets just say "game1", "game2", "game3", and "game4". The plugin, badly written, sends registerEvent and unregisterEvent packets every time a game ends - lets say a game ends every 20 minutes.
    That can accumulate a little bit of bandwidth, and it'd be nearly pointless especially if the ^game(1-4)$ channels are the only channels in use.
    Maybe there can be a registerForAll?
  3. Coelho

    Coelho Software Engineer Staff Member Administrator Maintainer

    This is designed to save bandwidth usage by making it so the server does not receive data it does not require. The amount saved would be likely 1000* (no, really) the amount used: even if the plugin is bad.

    Now an issue would be someone who listens for messages with no filters (requiring the server to listen for all message requests and completely breaking the point of this optimization).
  4. boboman13

    boboman13 Member Contributor

    @Coelho, none of the plugins currently implement this behavior and would cause the improvement to be slight if at all.

    Also, what would be the point of saving bandwidth usage? Most smaller servers use Multicraft or another managed solution, and larger systems typically have one or more dedicated servers, and if you have multiple in most cases you would have them in the same rack, allowing for same-network connections which doesn't cost bandwidth. In my opinion, though cutting back everywhere helps, wouldn't the bandwidth not necessarily be the top thing to cut back on?
  5. Coelho

    Coelho Software Engineer Staff Member Administrator Maintainer

    Yes, this optimization will not work for any active plugins. The only plugins that will receive the optimization are newer plugins, and only if you are using newer plugins.

    Bandwidth usage when it becomes large enough will result in high CPU usage on endpoint servers due to an excessive amount of received data.
  6. boboman13

    boboman13 Member Contributor

    I would probably say that the CPU usage wouldn't be necessarily as high as you would expect; I would estimate that 99% of servers don't use the messaging system beyond once per second.
    Does Go implement a way for us to check what processes use the most CPU? Maybe we can check other areas and cut back on CPU usage in there. However, I still support the idea.

    Edit: profiling with Go, after 10 seconds of googling
    Last edited: Jan 10, 2014
  7. Coelho

    Coelho Software Engineer Staff Member Administrator Maintainer

    Servers with an absurd amount of players (and sub-servers) will be consuming a lot of processing time on messages dependent on how often they are used. Not to mention, we give plugins the ability to send messages to all servers using the Connect server as a relay.

    Sure, we are not going to use *too much* CPU usage ever, but it's going to become noticeable after some point. Profiling will not help this: the code is already as optimal as can be. The solution is the heighten to event system so events are not received when not necessary (this proposition).
    • Like Like x 1
  8. confuser

    confuser Member Resource Contributor

    From a plugin point of view, it would be nice to have this kind of simple interface:

    public void onMessage(MessageEvent event) {

    That way the rather tedious event.getChannel().equals("achannel") statements within every method can be avoided.

    Renaming @EventListener to @ConnectEventHandler would bring some consistency between normal bukkit event handlers which may help new devs to lilypad.

    Like with anything new, it'll take some time to adopt so I wouldn't worry too much about option 2 for plugins that don't have filters. At the end of the day, it is up to the owner to ensure the plugins they are using are performant for their setup.
  9. Coelho

    Coelho Software Engineer Staff Member Administrator Maintainer

    I'm trying to find a way that we can use this for all events other than just MessageEvent. Otherwise the ability to define a channel in the ChannelEventHandler class is a little too specific. That's why I had the idea of filters but in every situation except for channels they prove fairly useless.

    Maybe something like this?
    Code (text):

    @MessageEventParams( connect = "mychannel" )
    public void onMessage(MessageEvent event) {
    So if you define MessageEventParams in conjunction with the EventHandler it'll take the params into account. But maybe that's still too specific implementation-wise.
  10. confuser

    confuser Member Resource Contributor

    Not a big fan of the double annotations to be honest.

    A filter for example lilyPad.registerChannelListener("channelName", new class()) could work. It's specific as is quite obvious as to what it does. The only downside would be that you're then forcing developers to have an entire class per channel. Personally that seems like a cleaner way however I'm sure many would disagree.

    I'm not sure what the issue with defining a channel within the ChannelEventHandler class is (haven't looked at the code) but the naming of the class makes it sound specific to channels (if I've understood correctly).

    If it's not a message event, ignore the channel annotation option. In fact, for flexibility, perhaps even the option to specify the user as well as the channel. Completely optional of course, but could be useful in some scenarios.

    @EventListener(channel="achannel", users="master,slave,etc")
  11. Coelho

    Coelho Software Engineer Staff Member Administrator Maintainer

    Or maybe something like this?
    Code (java):

    @ConnectEventHandler( params = MessageEventParams.channel("channelhere") )
    public void onMessage(MessageEvent event) {
    I think this looks pretty good.
  12. confuser

    confuser Member Resource Contributor

    That looks good :) What kind of uses other than channel would MessageEventParams have?
  13. Coelho

    Coelho Software Engineer Staff Member Administrator Maintainer

    I don't believe it would have any other uses. From there we can just create more params for each type of event (but the only one worth it as of the moment is a MessageEvent).

    The next course of action is to implement this model, and start pushing warning messages into the console output whenever the old "registerMessageEventListener" functions (etc) are used. I would also like to push a warning message if you register a MessageEvent without MessageEventParams (as this is not optimal).

    Then, once the API specification has set it's course for awhile, we'll remove the deprecated methods (they've been deprecated for what, 6 months now?) and start to implement the "registerEvent" and "unregisterEvent" packets to be sent to the Connect server for optimization.
    • Like Like x 1

Share This Page