You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by Emmanuel Lécharny <el...@symas.com> on 2015/07/21 17:52:59 UTC
[Studio] Inneficient loop while cliking on an Entry's value in the
browser
Hi !
I'm facing a very innefficient piece of code while trying to play around
teh OpenLDAP ACl editor. Here is what I do, and what I see.
- I connect to an OpenLDAP remote server
- I pick a MDB database entry (but that could be any entry which contain
the olcAccess attributeType (every ObjectClass inheriting from
olcDatabaseConfig, ie, any backend).
- I open it, and get all the attributes and values in it
- assuming there is at least one olcAccess instance in this entry, then
the OpenLdapAclValueWithContext constructor is called 28 times !
The reason is that we are iterating across all the TreeViexer
selectionChangeListeners, and we have many (62... One per editor).
This starts in :
protected void fireSelectionChanged(final SelectionChangedEvent event) {
Object[] listeners = selectionChangedListeners.getListeners();
for (Object listener : listeners) {
final ISelectionChangedListener l =
(ISelectionChangedListener) listener;
SafeRunnable.run(new SafeRunnable() {
@Override
public void run() {
l.selectionChanged(event); ---> This get called as
many times as we have listeners
}
});
}
}
Each loop invokes the EntryEditorActionProxy.selectionChanged() method,
then the BrowserActionProxy.updateAction(), which calls
OpenEditorAction.isEnabled(), which calls the
EntryEditorWidgetCellModifier.canModify() method, which tries to figure
out what is the best editor for the selected value, and finally call the
getRawValue() of the class used to edit the value.
In my case, it ends up calling the ACL parser as many times we have
listeners (28 times), even if the listeners are totally unrelated.
Here is the stack trace :
Thread [main] (Suspended (breakpoint at line 143 in
OpenLdapAclValueEditor))
OpenLdapAclValueEditor.getRawValue(IValue) line: 143
EntryEditorWidgetCellModifier.canModify(Object, String) line: 79
OpenEditorAction.isEnabled() line: 128
EntryEditorActionProxy(BrowserActionProxy).updateAction() line: 248
EntryEditorActionProxy(BrowserActionProxy).selectionChanged(SelectionChangedEvent)
line: 237
Viewer$2.run() line: 163
SafeRunner.run(ISafeRunnable) line: 42
JFaceUtil$1.run(ISafeRunnable) line: 50
SafeRunnable.run(ISafeRunnable) line: 178
>>>>TreeViewer(Viewer).fireSelectionChanged(SelectionChangedEvent) line:
160<<<<---------------- Starting loop
TreeViewer(StructuredViewer).updateSelection(ISelection) line: 2171
TreeViewer(StructuredViewer).handleSelect(SelectionEvent) line: 1202
StructuredViewer$4.widgetSelected(SelectionEvent) line: 1231
OpenStrategy.fireSelectionEvent(SelectionEvent) line: 242
OpenStrategy.access$4(OpenStrategy, SelectionEvent) line: 236
OpenStrategy$1.handleEvent(Event) line: 408
EventTable.sendEvent(Event) line: 84
Display.sendEvent(EventTable, Event) line: 4199
Tree(Widget).sendEvent(Event) line: 1467
Tree(Widget).sendEvent(int, Event, boolean) line: 1490
Tree(Widget).sendEvent(int, Event) line: 1475
Tree(Widget).notifyListeners(int, Event) line: 1279
Display.runDeferredEvents() line: 4042
Display.applicationNextEventMatchingMask(long, long, long, long,
long, long) line: 4996
Display.applicationProc(long, long, long, long, long, long) line:
5378
OS.objc_msgSendSuper(objc_super, long, long) line: not available
[native method]
Tree(Widget).callSuper(long, long, long) line: 221
Tree(Widget).mouseDownSuper(long, long, long) line: 1101
Tree.mouseDownSuper(long, long, long) line: 2064
Tree(Widget).mouseDown(long, long, long) line: 1093
Tree(Control).mouseDown(long, long, long) line: 2563
Tree.mouseDown(long, long, long) line: 2032
Display.windowProc(long, long, long) line: 5638
OS.objc_msgSendSuper(objc_super, long, long) line: not available
[native method]
Shell(Widget).callSuper(long, long, long) line: 221
Shell(Widget).windowSendEvent(long, long, long) line: 2105
Shell.windowSendEvent(long, long, long) line: 2329
Display.windowProc(long, long, long) line: 5702
OS.objc_msgSendSuper(objc_super, long, long) line: not available
[native method]
Display.applicationSendEvent(long, long, long) line: 5139
Display.applicationProc(long, long, long) line: 5288
OS.objc_msgSend(long, long, long) line: not available [native
method]
NSApplication.sendEvent(NSEvent) line: 128
Display.readAndDispatch() line: 3666
PartRenderingEngine$9.run() line: 1151
Realm.runWithDefault(Realm, Runnable) line: 332
PartRenderingEngine.run(MApplicationElement, IEclipseContext) line:
1032
E4Workbench.createAndRunUI(MApplicationElement) line: 148
Workbench$5.run() line: 636
Realm.runWithDefault(Realm, Runnable) line: 332
Workbench.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 579
PlatformUI.createAndRunWorkbench(Display, WorkbenchAdvisor) line:
150
Application.start(IApplicationContext) line: 51
EclipseAppHandle.run(Object) line: 196
EclipseAppLauncher.runApplication(Object) line: 134
EclipseAppLauncher.start(Object) line: 104
EclipseStarter.run(Object) line: 380
EclipseStarter.run(String[], Runnable) line: 235
NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not
available [native method]
NativeMethodAccessorImpl.invoke(Object, Object[]) line: 62
DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 43
Method.invoke(Object, Object...) line: 497
Main.invokeFramework(String[], URL[]) line: 648
Main.basicRun(String[]) line: 603
Main.run(String[]) line: 1465
Main.main(String[]) line: 1438
The question would be : how can we avoid such thing to happen ?
Re: [Studio] Inneficient loop while cliking on an Entry's value in
the browser
Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 22/07/15 01:10, Stefan Seelmann a écrit :
> I did a change here: http://svn.apache.org/r1692199
>
> As you already found out whenever a value is selected in the entry
> editor all the value editors (or better their proxy) are checked if they
> are enabled. Within this isEnabled() check these two lines checked the
> "current/best" editor if the value is editable. So in case the if the an
> olcAccess attribute is selected the OpenLdapAclValueEditor was asked 28
> times.
>
> I think this check is not required, but maybe it is from the
> SearchResultEditorCellModifier, I'm not totally sure. Let's first see
> the Jenkins build result tonight.
>
> Anyway, there is much room for improvement.
>
> We call "IValueEditor.getRawValue() != null" very often just to find out
> if the editor can handle a value.Getting the raw value is sometimes
> cheap (just return a string as is) but sometimes quite expensive. Maybe
> an additional "canHanlde()" method would help.
I wonder if it's the right approach. That may aleviate the load, but we
still have a lot of listeners registred...
I was thinking about another approach : just register one single
listener, which would find the right proxy on the fly (ie, when the
selection is done). That requires a bit of work, as we will have to
change the way we register the proxies, and to create this specific
selection listener, whch will contain the list of editor proxies.
I have debugged the code a few of hours yesterday, but it's not easy, as
they are a lots of layers I don't know.
I'll continue my investigation today.
Re: [Studio] Inneficient loop while cliking on an Entry's value in
the browser
Posted by Stefan Seelmann <ma...@stefan-seelmann.de>.
I did a change here: http://svn.apache.org/r1692199
As you already found out whenever a value is selected in the entry
editor all the value editors (or better their proxy) are checked if they
are enabled. Within this isEnabled() check these two lines checked the
"current/best" editor if the value is editable. So in case the if the an
olcAccess attribute is selected the OpenLdapAclValueEditor was asked 28
times.
I think this check is not required, but maybe it is from the
SearchResultEditorCellModifier, I'm not totally sure. Let's first see
the Jenkins build result tonight.
Anyway, there is much room for improvement.
We call "IValueEditor.getRawValue() != null" very often just to find out
if the editor can handle a value.Getting the raw value is sometimes
cheap (just return a string as is) but sometimes quite expensive. Maybe
an additional "canHanlde()" method would help.
Kind Regards,
Stefan
Re: [Studio] Inneficient loop while cliking on an Entry's value in
the browser
Posted by Stefan Seelmann <ma...@stefan-seelmann.de>.
Hi Emmanuel,
for sure that needs to be improved, I looked and debuged for 1 hour, but
right now I don't have a real idea and solution.
Kind Regards,
Stefan
Re: [Studio] Inneficient loop while cliking on an Entry's value in
the browser
Posted by Emmanuel Lécharny <el...@gmail.com>.
FTR, the listeners are registred in the
SingleTableEntryEditor.createPartControl() method :
actionGroup = new EntryEditorActionGroup( this );
-> super( entryEditor.getMainWidget(), entryEditor.getConfiguration() );
-> for ( int i = 0; i < openValueEditorActionProxies.length; i++ )
{
openValueEditorActionProxies[i] = new
EntryEditorActionProxy( viewer, new OpenEditorAction( viewer,
valueEditorManager, valueEditors[i], this ) );
-> EntryEditorActionProxy()
-> BrowserActionProxy()
-> selectionProvider.addSelectionChangedListener( this );
We have 24 elements in the openValueEditorActionProxies array, because
we have 24 possible editors :
org.apache.directory.studio.valueeditors.InPlaceTextValueEditor@3fd21272,
org.apache.directory.studio.valueeditors.TextValueEditor@4b4bc73d,
org.apache.directory.studio.valueeditors.HexValueEditor@4cb91fa4,
org.apache.directory.studio.valueeditors.bool.InPlaceBooleanValueEditor@12aa2cca,
org.apache.directory.studio.valueeditors.image.ImageValueEditor@3dc4ed6f,
org.apache.directory.studio.valueeditors.adtime.ActiveDirectoryTimeValueEditor@2823b7c5,
org.apache.directory.studio.valueeditors.administrativerole.AdministrativeRoleValueEditor@2ce3d95a,
org.apache.directory.studio.valueeditors.objectclass.ObjectClassValueEditor@6f6c6c70,
org.apache.directory.studio.valueeditors.integer.IntegerValueEditor@75d95b67,
org.apache.directory.studio.valueeditors.dn.DnValueEditor@40f15786,
org.apache.directory.studio.valueeditors.msad.InPlaceMsAdObjectSidValueEditor@72ea6fcb,
org.apache.directory.studio.aciitemeditor.valueeditors.SubtreeValueEditor@7ee9920a,
org.apache.directory.studio.valueeditors.password.PasswordValueEditor@4154ecd3,
org.apache.directory.studio.valueeditors.uuid.InPlaceUuidValueEditor@b53ce18,
org.apache.directory.studio.valueeditors.time.GeneralizedTimeValueEditor@2a666c8e,
org.apache.directory.studio.valueeditors.address.AddressValueEditor@18adc20a,
org.apache.directory.studio.valueeditors.oid.InPlaceOidValueEditor@7e7987b8,
org.apache.directory.studio.valueeditors.msad.InPlaceMsAdObjectGuidValueEditor@6bcfb561,
org.apache.directory.studio.valueeditors.certificate.CertificateValueEditor@7254a68a,
org.apache.directory.studio.aciitemeditor.ACIItemValueEditor@49f9758,
org.apache.directory.studio.openldap.config.acl.editor.OpenLdapAclValueEditor@36e98434,
org.apache.directory.studio.valueeditors.MultivaluedValueEditor@209b4b35,
org.apache.directory.studio.valueeditors.EntryValueEditor@77acc95a,
org.apache.directory.studio.valueeditors.RenameValueEditor@ff1a5a0]
Ideally speaking, the listener to be called should be the one associated
with the selected AttributeType. An option would be to register an
encaspulating listener that filters the editor to call depending on teh
AttributeType.
Le 21/07/15 17:52, Emmanuel Lécharny a écrit :
> Hi !
>
> I'm facing a very innefficient piece of code while trying to play around
> teh OpenLDAP ACl editor. Here is what I do, and what I see.
>
> - I connect to an OpenLDAP remote server
> - I pick a MDB database entry (but that could be any entry which contain
> the olcAccess attributeType (every ObjectClass inheriting from
> olcDatabaseConfig, ie, any backend).
> - I open it, and get all the attributes and values in it
> - assuming there is at least one olcAccess instance in this entry, then
> the OpenLdapAclValueWithContext constructor is called 28 times !
>
> The reason is that we are iterating across all the TreeViexer
> selectionChangeListeners, and we have many (62... One per editor).
>
> This starts in :
>
> protected void fireSelectionChanged(final SelectionChangedEvent event) {
> Object[] listeners = selectionChangedListeners.getListeners();
> for (Object listener : listeners) {
> final ISelectionChangedListener l =
> (ISelectionChangedListener) listener;
> SafeRunnable.run(new SafeRunnable() {
> @Override
> public void run() {
> l.selectionChanged(event); ---> This get called as
> many times as we have listeners
> }
> });
> }
> }
>
> Each loop invokes the EntryEditorActionProxy.selectionChanged() method,
> then the BrowserActionProxy.updateAction(), which calls
> OpenEditorAction.isEnabled(), which calls the
> EntryEditorWidgetCellModifier.canModify() method, which tries to figure
> out what is the best editor for the selected value, and finally call the
> getRawValue() of the class used to edit the value.
>
> In my case, it ends up calling the ACL parser as many times we have
> listeners (28 times), even if the listeners are totally unrelated.
>
> Here is the stack trace :
>
> Thread [main] (Suspended (breakpoint at line 143 in
> OpenLdapAclValueEditor))
> OpenLdapAclValueEditor.getRawValue(IValue) line: 143
> EntryEditorWidgetCellModifier.canModify(Object, String) line: 79
> OpenEditorAction.isEnabled() line: 128
> EntryEditorActionProxy(BrowserActionProxy).updateAction() line: 248
>
> EntryEditorActionProxy(BrowserActionProxy).selectionChanged(SelectionChangedEvent)
> line: 237
> Viewer$2.run() line: 163
> SafeRunner.run(ISafeRunnable) line: 42
> JFaceUtil$1.run(ISafeRunnable) line: 50
> SafeRunnable.run(ISafeRunnable) line: 178
>>>>> TreeViewer(Viewer).fireSelectionChanged(SelectionChangedEvent) line:
> 160<<<<---------------- Starting loop
> TreeViewer(StructuredViewer).updateSelection(ISelection) line: 2171
> TreeViewer(StructuredViewer).handleSelect(SelectionEvent) line: 1202
> StructuredViewer$4.widgetSelected(SelectionEvent) line: 1231
> OpenStrategy.fireSelectionEvent(SelectionEvent) line: 242
> OpenStrategy.access$4(OpenStrategy, SelectionEvent) line: 236
> OpenStrategy$1.handleEvent(Event) line: 408
> EventTable.sendEvent(Event) line: 84
> Display.sendEvent(EventTable, Event) line: 4199
> Tree(Widget).sendEvent(Event) line: 1467
> Tree(Widget).sendEvent(int, Event, boolean) line: 1490
> Tree(Widget).sendEvent(int, Event) line: 1475
> Tree(Widget).notifyListeners(int, Event) line: 1279
> Display.runDeferredEvents() line: 4042
> Display.applicationNextEventMatchingMask(long, long, long, long,
> long, long) line: 4996
> Display.applicationProc(long, long, long, long, long, long) line:
> 5378
> OS.objc_msgSendSuper(objc_super, long, long) line: not available
> [native method]
> Tree(Widget).callSuper(long, long, long) line: 221
> Tree(Widget).mouseDownSuper(long, long, long) line: 1101
> Tree.mouseDownSuper(long, long, long) line: 2064
> Tree(Widget).mouseDown(long, long, long) line: 1093
> Tree(Control).mouseDown(long, long, long) line: 2563
> Tree.mouseDown(long, long, long) line: 2032
> Display.windowProc(long, long, long) line: 5638
> OS.objc_msgSendSuper(objc_super, long, long) line: not available
> [native method]
> Shell(Widget).callSuper(long, long, long) line: 221
> Shell(Widget).windowSendEvent(long, long, long) line: 2105
> Shell.windowSendEvent(long, long, long) line: 2329
> Display.windowProc(long, long, long) line: 5702
> OS.objc_msgSendSuper(objc_super, long, long) line: not available
> [native method]
> Display.applicationSendEvent(long, long, long) line: 5139
> Display.applicationProc(long, long, long) line: 5288
> OS.objc_msgSend(long, long, long) line: not available [native
> method]
> NSApplication.sendEvent(NSEvent) line: 128
> Display.readAndDispatch() line: 3666
> PartRenderingEngine$9.run() line: 1151
> Realm.runWithDefault(Realm, Runnable) line: 332
> PartRenderingEngine.run(MApplicationElement, IEclipseContext) line:
> 1032
> E4Workbench.createAndRunUI(MApplicationElement) line: 148
> Workbench$5.run() line: 636
> Realm.runWithDefault(Realm, Runnable) line: 332
> Workbench.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 579
> PlatformUI.createAndRunWorkbench(Display, WorkbenchAdvisor) line:
> 150
> Application.start(IApplicationContext) line: 51
> EclipseAppHandle.run(Object) line: 196
> EclipseAppLauncher.runApplication(Object) line: 134
> EclipseAppLauncher.start(Object) line: 104
> EclipseStarter.run(Object) line: 380
> EclipseStarter.run(String[], Runnable) line: 235
> NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not
> available [native method]
> NativeMethodAccessorImpl.invoke(Object, Object[]) line: 62
> DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 43
> Method.invoke(Object, Object...) line: 497
> Main.invokeFramework(String[], URL[]) line: 648
> Main.basicRun(String[]) line: 603
> Main.run(String[]) line: 1465
> Main.main(String[]) line: 1438
>
>
> The question would be : how can we avoid such thing to happen ?