Bug 496006 - Avoid invalidating the Java service cache on each properties-specific evaluation
Summary: Avoid invalidating the Java service cache on each properties-specific evaluation
Status: CLOSED FIXED
Alias: None
Product: Sirius
Classification: Modeling
Component: Properties (show other bugs)
Version: 4.0.0   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 4.1.0   Edit
Assignee: Pierre-Charles David CLA
QA Contact:
URL: https://tuleap.eclipse.org/plugins/tr...
Whiteboard:
Keywords: performance, triaged
Depends on:
Blocks: 495366
  Show dependency tree
 
Reported: 2016-06-13 10:21 EDT by Pierre-Charles David CLA
Modified: 2017-01-05 11:41 EST (History)
2 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Pierre-Charles David CLA 2016-06-13 10:21:01 EDT 
https://git.eclipse.org/r/#/c/67741/ identifed one of the main culprits in terms of performance when properties views are enabled: properties view require a specific service class (SiriusToolsService) to be registered in the session's interpreters to work, and Sirius does not currently offer any better way than to register it programmatically and then unregister it after use to avoid name clashes. This constant register/unregister has a huge performance impact.

The actual solution is not obvious, and several options are on the table right now. Some of them may be more general than just fixing the SiriusToolsService issue, but for now the scope is to fix the performance issue in the context of properties view. If we end up with a more general solution, so much the better, but it's not the main priority.

Some options:
* keep the register/unregister as it is, but change JavaExtensionManager to not invalidate its whole cache just because we unregistered a single service class. More generaly, tweak JavaExtensionManager so that register/unregister is as close as a "free" op as possible.
* never unregister the SiriusToolsService, but somehow ensure the services it provides do not risk clashing with other service classes in the session.

See also the commit message in https://git.eclipse.org/r/#/c/67741/ for other ideas.
Comment 1 Pierre-Charles David CLA 2016-09-15 08:36:23 EDT 
After analysis, the first approach is too impacting. At first glance it leads deep in AQL's ServiceStore, a critical piece of AQL's runtime which is optimized for lookup. Any change there is very risky.

We'll take the second approach: leave SiriusToolsService registered and the services it exposes visible to the whole session (easy), and refactor the said services to reduce the risk of clashes with user-provided services that could be signature-compatible.
Comment 2 Pierre-Charles David CLA 2016-09-15 10:11:28 EDT 
Some performance numbers: using an artificial example metamodel where elements have 60 features (a mix of booleans, ints, strings and non-containment references), applying the default rules to produce the properties of such an element takes around 660ms (as measured by the time taken in EEFTabbedPropertySheetPage.doSetInput(), and after several iterations to get stable results).

Simply avoiding the unregistration of SiriusToolServices, the same scenario takes about 420ms, so a speed improvement of 37%.
Comment 4 Eclipse Genie CLA 2016-10-02 09:59:46 EDT 
New Gerrit change created: https://git.eclipse.org/r/82332
Comment 5 Eclipse Genie CLA 2016-10-02 09:59:50 EDT 
New Gerrit change created: https://git.eclipse.org/r/82331
Comment 6 Eclipse Genie CLA 2016-10-02 09:59:51 EDT 
New Gerrit change created: https://git.eclipse.org/r/82330
Comment 10 Eclipse Genie CLA 2016-10-05 10:23:35 EDT 
New Gerrit change created: https://git.eclipse.org/r/82530
Comment 11 Eclipse Genie CLA 2016-10-05 10:23:38 EDT 
New Gerrit change created: https://git.eclipse.org/r/82529
Comment 12 Eclipse Genie CLA 2016-10-05 10:23:39 EDT 
New Gerrit change created: https://git.eclipse.org/r/82528
Comment 13 Eclipse Genie CLA 2016-10-05 10:23:41 EDT 
New Gerrit change created: https://git.eclipse.org/r/82527
Comment 14 Eclipse Genie CLA 2016-10-05 10:23:43 EDT 
New Gerrit change created: https://git.eclipse.org/r/82526
Comment 15 Eclipse Genie CLA 2016-10-05 10:23:48 EDT 
New Gerrit change created: https://git.eclipse.org/r/82532
Comment 16 Eclipse Genie CLA 2016-10-05 10:23:49 EDT 
New Gerrit change created: https://git.eclipse.org/r/82531
Comment 25 Pierre-Charles David CLA 2016-10-06 08:17:59 EDT 
Fixed. There's nothing specific to validate here, as it is mainly internal changes for optimization purposes. It should have had no visible impact except on speed.
Comment 26 Pierre-Charles David CLA 2016-10-18 11:08:33 EDT 
Available in Sirius 4.1.0, see https://wiki.eclipse.org/Sirius/4.1.0 for details.
Comment 28 Eclipse Genie CLA 2016-12-29 10:24:09 EST 
New Gerrit change created: https://git.eclipse.org/r/87805
Comment 29 Eclipse Genie CLA 2017-01-02 10:50:14 EST 
New Gerrit change created: https://git.eclipse.org/r/87877
Comment 30 Eclipse Genie CLA 2017-01-02 10:50:59 EST 
New Gerrit change created: https://git.eclipse.org/r/87878