Code::Blocks Forums

Developer forums (C::B DEVELOPMENT STRICTLY!) => Development => Topic started by: mandrav on July 05, 2007, 02:18:20 pm

Title: Feedback needed for change in plugins' event handling
Post by: mandrav on July 05, 2007, 02:18:20 pm
We are considering changing the way C::B events are forwarded to plugins for processing so as to cut down the number of events flowing around.
In case you hadn't noticed it, with the current system, a single event might end up many times in the event handler of one plugin. This is what we 're trying to solve.

The proposed approach looks like this:

The above process needs minimum code changes in both the SDK as well as the plugins. Basically, from the plugin writer's point of view, we just move the event handling registration from the plugin's event table to its OnAttach() method. Really a minimum change.

By going with the above change, we can guarantee that each registered event handler will be called exactly once for each event entering the queue.
We also minimize the overhead because a normal wxEvent handled by the event table will have to enter the event processing queue and be delivered where it is needed. With this change, we 're talking about a single loop over only the registered handlers and that's it.

We should be able to gain in speed and responsiveness.
One more great advantage is that by using this approach, we could even get feedback from the event handlers because the event is passed along synchronously. Which means after we return from a call to an event handler, we could examine the event for raised flags, changed contents etc.

We 'd like to have feedback on this proposal. Please share your views :)
Title: Re: Feedback needed for change in plugins' event handling
Post by: thomas on July 05, 2007, 02:25:56 pm
It's important to point out that we're talking of CodeBlocksEvents here, i.e. not of wxWidgets GUI or timer events, for example.
The latter might cause serious issues with the GUI hanging for half a second or things like that, if run synchronously.

My solution to the problem would have been to implement something like callbacks (rather this-pointer calls, actually) and kicking events for this matter alltogether. While that would probably be more efficient, it would require a lot of code changes, so Yiannis' solution is likely preferrable.
Title: Re: Feedback needed for change in plugins' event handling
Post by: mandrav on July 05, 2007, 02:36:41 pm
Quote from: thomas on July 05, 2007, 02:25:56 pm
It's important to point out that we're talking of CodeBlocksEvents here, i.e. not of wxWidgets GUI or timer events, for example.
The latter might cause serious issues with the GUI hanging for half a second or things like that, if run synchronously.

Correct.

Quote from: thomas on July 05, 2007, 02:25:56 pm
My solution to the problem would have been to implement something like callbacks (rather this-pointer calls, actually) and kicking events for this matter alltogether. While that would probably be more efficient, it would require a lot of code changes, so Yiannis' solution is likely preferrable.

Well, this-pointer calls will be used so we 're really talking about the same thing :).
Title: Re: Feedback needed for change in plugins' event handling
Post by: JGM on July 05, 2007, 02:50:06 pm
Quote from: mandrav on July 05, 2007, 02:18:20 pm
The proposed approach looks like this:
  • A new function is added: PluginManager::RegisterEventSink(const wxEventType evtType, void(cbPlugin::*handler)(CodeBlocksEvent&))

Could you give a concrete example of how the code will look like on the attach function, with an actual event?
Title: Re: Feedback needed for change in plugins' event handling
Post by: mandrav on July 05, 2007, 03:11:46 pm
Quote from: JGM on July 05, 2007, 02:50:06 pm
Quote from: mandrav on July 05, 2007, 02:18:20 pm
The proposed approach looks like this:
  • A new function is added: PluginManager::RegisterEventSink(const wxEventType evtType, void(cbPlugin::*handler)(CodeBlocksEvent&))

Could you give a concrete example of how the code will look like on the attach function, with an actual event?


Old sample code:

Code
BEGIN_EVENT_TABLE(SomePlugin, cbPlugin)
...
EVT_EDITOR_SAVE(SomePlugin::OnSave)
...
END_EVENT_TABLE()


New sample code:

Code
BEGIN_EVENT_TABLE(SomePlugin, cbPlugin)
...
// remove EVT_EDITOR_SAVE entry
...
END_EVENT_TABLE()

void SomePlugin::OnAttach()
{
...
PluginManager* pm = Manager::Get()->GetPluginManager();
pm->RegisterEventSink(cbEVT_EDITOR_SAVE, &SomePlugin::OnSave);
...
}

That's more or less what needs to change in plugins. Let me stress once more that we 're only talking about Code::Blocks events here...
Title: Re: Feedback needed for change in plugins' event handling
Post by: David Perfors on July 05, 2007, 03:15:36 pm
It is indeed a lot better then the macro stuff of Code::Blocks. It is strange that wx can't guarantee that the event isn't fired once... (I think you are saying that indirectly..)
Title: Re: Feedback needed for change in plugins' event handling
Post by: dmoore on July 05, 2007, 03:18:31 pm
sounds good to me.

will you also set this up for all of the plugin virtuals (other than OnAttach and OnRelease)? The registration of methods like BuildMenu and BuildModuleMenu could then go into the boiler plate for the plugin wizard, which will be a nice signpost for new users.
Title: Re: Feedback needed for change in plugins' event handling
Post by: mandrav on July 05, 2007, 03:41:56 pm
Quote from: dmoore on July 05, 2007, 03:18:31 pm
sounds good to me.

will you also set this up for all of the plugin virtuals (other than OnAttach and OnRelease)? The registration of methods like BuildMenu and BuildModuleMenu could then go into the boiler plate for the plugin wizard, which will be a nice signpost for new users.

BuildMenu and friends are not events so no, they won't be handled this way.
But after this system is implemented, we could add a new page to the plugin wizard to allow the user to (multiple-)select from all the core events and add code appropriately.
Title: Re: Feedback needed for change in plugins' event handling
Post by: dmoore on July 05, 2007, 04:07:14 pm
Quote from: mandrav on July 05, 2007, 03:41:56 pm
BuildMenu and friends are not events so no, they won't be handled this way.

isn't the distinction artificial? (i.e. buildmenu, buildmodulemenu etc could be setup as events, passing the menu data in the event structure). However, given the time and minimal breakage constraints, I understand the decision (I just like consistency).

Quote
But after this system is implemented, we could add a new page to the plugin wizard to allow the user to (multiple-)select from all the core events and add code appropriately.

:)
Title: Re: Feedback needed for change in plugins' event handling
Post by: rickg22 on July 05, 2007, 09:38:18 pm
I support the motion. For one reason: Rogue plugins.

Have you noticed, for example, that many plugins have the if(isAttached()) code included in their handlers? What if i make a plugin virus and i don't care if the plugin's attached or not, and execute the event handling? So yes, we need to activate the event handling only AFTER the plugin's attached. But please unregister the events automatically so the plugin writer won't have to unregister their plugins.

One thing, tho. I don't think "RegisterEventSink" is an appropriate name. How about "RegisterCBEventHandler"? :)
Title: Re: Feedback needed for change in plugins' event handling
Post by: mandrav on July 05, 2007, 09:47:45 pm
Quote
So yes, we need to activate the event handling only AFTER the plugin's attached.

That's how the EXISTING implementation works, a few years now.
Title: Re: Feedback needed for change in plugins' event handling
Post by: rickg22 on July 05, 2007, 10:47:52 pm
Oh, ok :)
Title: Re: Feedback needed for change in plugins' event handling
Post by: killerbot on July 06, 2007, 02:40:30 pm
sounds good, ok for me; let the performance jump forward
Title: Re: Feedback needed for change in plugins' event handling
Post by: mandrav on July 06, 2007, 03:31:02 pm
OK people,

I just committed the changes. You should note no difference on the operation of Code::Blocks (except maybe from speed in some places - but then again you should know where to look for). The important part is that everything in our repository is updated (sdk, app, core and contrib plugins).

For plugin writers (and anyone else wanting to know about the changes), I 've written a small article on the subject: Code::Blocks SDK events (http://wiki.codeblocks.org/index.php?title=Code::Blocks_SDK_events).
Title: Re: Feedback needed for change in plugins' event handling
Post by: dmoore on July 06, 2007, 04:46:50 pm
do you want this in the wiki entry?

Code
'''Events the must be registered with the RegisterEventSink'''
''app events''
* cbEVT_APP_STARTUP_DONE
* cbEVT_APP_START_SHUTDOWN
* cbEVT_APP_UPDATE_TITLE

''plugin events''
* cbEVT_PLUGIN_ATTACHED
* cbEVT_PLUGIN_RELEASED
* cbEVT_PLUGIN_INSTALLED
* cbEVT_PLUGIN_UNINSTALLED

''editor events''
* cbEVT_EDITOR_CLOSE
* cbEVT_EDITOR_OPEN
* cbEVT_EDITOR_ACTIVATED
* cbEVT_EDITOR_DEACTIVATED
* cbEVT_EDITOR_SAVE
* cbEVT_EDITOR_MODIFIED
* cbEVT_EDITOR_TOOLTIP
* cbEVT_EDITOR_TOOLTIP_CANCEL
* cbEVT_EDITOR_BREAKPOINT_ADD
* cbEVT_EDITOR_BREAKPOINT_EDIT
* cbEVT_EDITOR_BREAKPOINT_DELETE
* cbEVT_EDITOR_UPDATE_UI

''project events''
* cbEVT_PROJECT_CLOSE
* cbEVT_PROJECT_OPEN
* cbEVT_PROJECT_SAVE
* cbEVT_PROJECT_ACTIVATE
* cbEVT_PROJECT_FILE_ADDED
* cbEVT_PROJECT_FILE_REMOVED
* cbEVT_PROJECT_POPUP_MENU
* cbEVT_PROJECT_TARGETS_MODIFIED
* cbEVT_PROJECT_RENAMED
* cbEVT_WORKSPACE_CHANGED

''dockable windows''
* cbEVT_ADD_DOCK_WINDOW: request app to add and manage a docked window
* cbEVT_REMOVE_DOCK_WINDOW: request app to stop managing a docked window
* cbEVT_SHOW_DOCK_WINDOW: request app to show a docked window
* cbEVT_HIDE_DOCK_WINDOW: request app to hide a docked window
* cbEVT_DOCK_WINDOW_VISIBILITY: app notifies that a docked window has been hidden/shown to actually find out its state use IsWindowReallyShown(event.pWindow);

''layout related events''
* cbEVT_SWITCH_VIEW_LAYOUT: request app to switch view layout
* cbEVT_SWITCHED_VIEW_LAYOUT: app notifies that a new layout has been applied

''main menubar creation''
* cbEVT_MENUBAR_CREATE_BEGIN: app notifies that the menubar is started being (re)created
* cbEVT_MENUBAR_CREATE_END: app notifies that the menubar (re)creation ended

''compiler-related events''
* cbEVT_COMPILER_STARTED
* cbEVT_COMPILER_FINISHED

''debugger-related events''
* cbEVT_DEBUGGER_STARTED
* cbEVT_DEBUGGER_PAUSED
* cbEVT_DEBUGGER_FINISHED

'''Events processed on wxWidgets EVENT TABLES and not registered using RegisterEventSink'''
''pipedprocess events''
* cbEVT_PIPEDPROCESS_STDOUT
* cbEVT_PIPEDPROCESS_STDERR
* cbEVT_PIPEDPROCESS_TERMINATED

''thread-pool events''
* cbEVT_THREADTASK_STARTED
* cbEVT_THREADTASK_ENDED
* cbEVT_THREADTASK_ALLDONE

Title: Re: Feedback needed for change in plugins' event handling
Post by: mandrav on July 06, 2007, 07:04:11 pm
Quote from: dmoore on July 06, 2007, 04:46:50 pm
do you want this in the wiki entry?

Code
'''Events the must be registered with the RegisterEventSink'''
''app events''
* cbEVT_APP_STARTUP_DONE
* cbEVT_APP_START_SHUTDOWN
* cbEVT_APP_UPDATE_TITLE

''plugin events''
* cbEVT_PLUGIN_ATTACHED
* cbEVT_PLUGIN_RELEASED
* cbEVT_PLUGIN_INSTALLED
* cbEVT_PLUGIN_UNINSTALLED

''editor events''
* cbEVT_EDITOR_CLOSE
* cbEVT_EDITOR_OPEN
* cbEVT_EDITOR_ACTIVATED
* cbEVT_EDITOR_DEACTIVATED
* cbEVT_EDITOR_SAVE
* cbEVT_EDITOR_MODIFIED
* cbEVT_EDITOR_TOOLTIP
* cbEVT_EDITOR_TOOLTIP_CANCEL
* cbEVT_EDITOR_BREAKPOINT_ADD
* cbEVT_EDITOR_BREAKPOINT_EDIT
* cbEVT_EDITOR_BREAKPOINT_DELETE
* cbEVT_EDITOR_UPDATE_UI

''project events''
* cbEVT_PROJECT_CLOSE
* cbEVT_PROJECT_OPEN
* cbEVT_PROJECT_SAVE
* cbEVT_PROJECT_ACTIVATE
* cbEVT_PROJECT_FILE_ADDED
* cbEVT_PROJECT_FILE_REMOVED
* cbEVT_PROJECT_POPUP_MENU
* cbEVT_PROJECT_TARGETS_MODIFIED
* cbEVT_PROJECT_RENAMED
* cbEVT_WORKSPACE_CHANGED

''dockable windows''
* cbEVT_ADD_DOCK_WINDOW: request app to add and manage a docked window
* cbEVT_REMOVE_DOCK_WINDOW: request app to stop managing a docked window
* cbEVT_SHOW_DOCK_WINDOW: request app to show a docked window
* cbEVT_HIDE_DOCK_WINDOW: request app to hide a docked window
* cbEVT_DOCK_WINDOW_VISIBILITY: app notifies that a docked window has been hidden/shown to actually find out its state use IsWindowReallyShown(event.pWindow);

''layout related events''
* cbEVT_SWITCH_VIEW_LAYOUT: request app to switch view layout
* cbEVT_SWITCHED_VIEW_LAYOUT: app notifies that a new layout has been applied

''main menubar creation''
* cbEVT_MENUBAR_CREATE_BEGIN: app notifies that the menubar is started being (re)created
* cbEVT_MENUBAR_CREATE_END: app notifies that the menubar (re)creation ended

''compiler-related events''
* cbEVT_COMPILER_STARTED
* cbEVT_COMPILER_FINISHED

''debugger-related events''
* cbEVT_DEBUGGER_STARTED
* cbEVT_DEBUGGER_PAUSED
* cbEVT_DEBUGGER_FINISHED

'''Events processed on wxWidgets EVENT TABLES and not registered using RegisterEventSink'''
''pipedprocess events''
* cbEVT_PIPEDPROCESS_STDOUT
* cbEVT_PIPEDPROCESS_STDERR
* cbEVT_PIPEDPROCESS_TERMINATED

''thread-pool events''
* cbEVT_THREADTASK_STARTED
* cbEVT_THREADTASK_ENDED
* cbEVT_THREADTASK_ALLDONE


Sure, go ahead and add them :).