eclipse-qvto / org.eclipse.qvto

Eclipse Public License 2.0
0 stars 1 forks source link

Chained compilation error feedback #943

Open eclipse-qvt-oml-bot opened 4 days ago

eclipse-qvt-oml-bot commented 4 days ago

| --- | --- | | Bugzilla Link | 486487 | | Status | REOPENED | | Importance | P3 normal | | Reported | Jan 25, 2016 11:11 EDT | | Modified | Dec 08, 2020 12:22 EDT | | See also | 567986 | | Reporter | Christopher Gerking |

Description

For a chain of module imports, the Diagnostic provided by the TransformationExecutor class does only indicate the most downstream compilation error (caused by importing another erroneous module). Thus, the module that actually causes the error remains unknown. This makes it extremely hard to detect the root cause, especially when the transformation is already deployed to a plugin. Instead, provide a DiagnosticChain which allows proper navigation to the root cause.

eclipse-qvt-oml-bot commented 4 days ago

By Christopher Gerking on Jun 29, 2016 09:02

(In reply to Christopher Gerking from comment #0)

For a chain of module imports, the Diagnostic provided by the TransformationExecutor class does only indicate the most downstream compilation error

Patch pushed to cgerking/486478. Now a QvtMessage is itself a DiagnosticChain and can refer to child diagnostics.

Commit ID: b161a71df2dbb4dc0c2342cb5540b90765c372fd

eclipse-qvt-oml-bot commented 4 days ago

By Christopher Gerking on Jul 31, 2018 03:59

Still pending review. Any chance to get this to master?

eclipse-qvt-oml-bot commented 4 days ago

By Ed Willink on Jul 31, 2018 04:11

When I rebase cgerking/486478 on master, it would appear that most and probably all of cgerking/486478 is on master already suggesting we just need a RESOLVED FIXED.

eclipse-qvt-oml-bot commented 4 days ago

By Christopher Gerking on Jul 31, 2018 04:30

(In reply to Ed Willink from comment #3)

When I rebase cgerking/486478 on master, it would appear that most and probably all of cgerking/486478 is on master already suggesting we just need a RESOLVED FIXED.

Right. Resolving as fixed.

eclipse-qvt-oml-bot commented 4 days ago

By Toni Siljamäki on Oct 18, 2020 17:35

Created attachment 284507 Test and debug info regarding the performance bug

QVTO Parser Performance Bug 2020-10-10.pdf

eclipse-qvt-oml-bot commented 4 days ago

By Toni Siljamäki on Oct 18, 2020 17:36

I just migrated my latest Papyrus-based DSML with customizations and the QVTO-based M2M+M2T Model Compiler from the Neon.2 to the 2020-06 Eclipse tooling. I have stayed with the ”latest stable” Neon.2 version since 2016, but now I got a perfect reason and opportunity to update to 2020-06.

During this work I found that a severe performance bug was introduced in the org.eclipse.m2m.qvt.oml version 3.7.0 for Oxygen.

Bug 486487 is listed in the Java-code-comments only in file QVTOCompiler.java. However… The bug 486487 title and Description gave me some hints of the reason to why I experience this severe performance bug in the parser/loader, but there are no leads to where the actual performance bug was introduced (in which Java code file(s)), so I have now done some extensive testing and debugging of the org.eclipse.m2m.qvt.oml plugin.

Comment 2-3 in bug 486487 tells that this code was actually released very long before it was formally reviewed.

In the QVTO-based Model Compiler I’m about to release to new users, I have disabled the problematic code in org.eclipse.m2m.qvt.oml and re-built this plugin, since I have only one chance to make a first good impression regarding tooling performance and efficiency for new users.

See attached file for my test-and-debug information.

NOTE: What I would need now is a new TransformationExecutor function to cleanupForReuse() so that a QVTO-based Model Compiler can be pre-loaded (loaded on Eclipse startup) and ”safely” reused for transformation of multiple DSL models without having to load and parse the QVTO transformations over-and-over again for every new DSL model to transform, in order to eliminate the ~2 seconds transformation startup parsing delay for users.

In my Model Compiler I am pre-loading the QVTO transformations on Elipse startup and I have disabled the current executor.cleanup(); function call.\ So… I can reuse the pre-loaded Model Compiler, but I do not know if there is any stuff that should be cleanupForReuse() between each run.

eclipse-qvt-oml-bot commented 4 days ago

By Ed Willink on Oct 19, 2020 04:09

The code was never formally reviewed, Christopher wrote it, Sergey rarely has time to participate, I lack the familiarity to do more that a 'coding standards' review. I pushed the code since it was long outstanding and seemingly ok.

You have clearly investigated carefully, but your PDF snippets are of very limited value beyond confirming that you spent non-trivial time. Thank you.

To progress we need a JUnit test and two preferably adjacent identified QVTo versions that behave well / badly. Probably 3.6.0/3.7.0. See Bug 567986.

Scanning the comments I wouldn't expect the extra QVTMessage inheritance of DiagnosticChain to make a measurable difference. Since DiagnosticChain is used by EMF too, perhaps some extra EMF activity has accidentally been triggered. Perhaps something not mentioned in the comments changed too.

Anyway, with a JUnit test and two versions it shouldn't be too difficult to profile one's way to the problem.

eclipse-qvt-oml-bot commented 4 days ago

By Christopher Gerking on Oct 19, 2020 05:46

(In reply to Ed Willink from comment #7)

Scanning the comments I wouldn't expect the extra QVTMessage inheritance of DiagnosticChain to make a measurable difference.

Indeed. Anyway, the change identified as a root cause of the degradation is due to bug 496181 (commit 210b966aa4aa9bfcd3cc0e55016757a2c045f5b0). So it does not even relate to DiagnosticChain and has nothing to do with this bug at all.

However I still cannot believe that Sergey's changes for bug 496181 cause such a performance degradation. It's just an additional validation.

Such problems rather remind me of bug 511765, which has been fixed by commit 06d96c4b890a53f8e078bb39a10655d5beab68ed. The problem was that HashMaps used to speed up a single compilation were never cleared, so the lookup used more and more time with every new compilation.

In my Model Compiler I am pre-loading the QVTO transformations on Elipse startup and I have disabled the current executor.cleanup(); function call.

If I get you right, you were invoking cleanup() although you want to reuse your executor? You shouldn't do this because obviously all caches will be cleared and the transformation requires full recompilation. It's no surprise that the performance degrades.

So without further information, this seems to be a user error. Please provide a repro.

eclipse-qvt-oml-bot commented 4 days ago

By Christopher Gerking on Oct 19, 2020 06:04

(In reply to Christopher Gerking from comment #8)

The problem was that HashMaps used to speed up a single compilation were never cleared, so the lookup used more and more time with every new compilation.

To be more precise, the time to lookup should obviously be constant in case of HashMap. Instead, the actual problem was that after several compilations, the maps used so much memory that the overall system performance collapsed.

(In reply to Toni Siljamäki from comment #6)

So… I can reuse the pre-loaded Model Compiler, but I do not know if there is any stuff that should be cleanupForReuse() between each run.

This seems to be a fundamental misunderstanding. You do not have to cleanup() your executor after a run, unless you want to deallocate its resources. But if you do, all the resources must obviously be reallocated on the next run, which takes time.

eclipse-qvt-oml-bot commented 4 days ago

By Toni Siljamäki on Dec 04, 2020 06:19

In order to eliminate the very long parsing times, I have commented out the problematic "parse local variables" code in QvtOperationalVisitorCS.java (see attached pdf) in my version of org.eclipse.m2m.qvt.oml.

I am now pre-loading the QVTO-based UML Model Compiler during Eclipse startup so that it runs immediately when started. (= no delay)

I have removed the earlier executor.cleanup(); so that I can reuse the pre-loaded Model Compiler.\ In the QVTO 3.6.0 version this cleanup was not an issue, but after 3.6.0 it triggers a 25sec recompilation of the .qvto transformations.\ I do not know if there is anything that should be cleaned out between each run, but now it starts and runs quickly. :)

My current problem is that a .qvto transform file which is open when the workspace is launched is never parsed.\ I must close the file and reopen it to get it parsed, sometimes I have to close and reopen it several times to get it parsed.\ Something is preventing the .qvto file from being parsed. See NOTE2-NOTE3 in attached slide #69.

In my development iterations I continously rebuild-and-run the QVTO-based Model Compiler plugin, ~30sec turnaround times = quite short, so the currently edited .qvto files not being parsed when the workspace is relaunched is a bit annoying. (not an issue in 3.6.0)

I have just tried the latest QVTO nightly, there is no change wrt the performance and parsing issues above.

eclipse-qvt-oml-bot commented 4 days ago

By Christopher Gerking on Dec 04, 2020 08:40

(In reply to Toni Siljamäki from comment #10)

In order to eliminate the very long parsing times, I have commented out the problematic "parse local variables" code in QvtOperationalVisitorCS.java (see attached pdf) in my version of org.eclipse.m2m.qvt.oml.

It's not problematic. It's a check for conflicting variables that reports potential problems (see bug 496181). Just because it doesn't report any problems for you doesn't mean it's useless.

I do not know if there is anything that should be cleaned out between each run, but now it starts and runs quickly. :)

Cleaning up shouldn't be necessary between two runs, so removing cleanup() is a good decision here.

However I still think that the root of your problem is something different. Without access to your example transformations, I can only guess that you are importing a lot of libraries? If so, please note that the default in Eclipse is import by extends, which causes the importing transformation to inherit all local variables from all (transitively) imported libraries. That's why the check for conflicting variables takes so long. So please try to change your transformation to imports by access, which will only make the library contents available without inheriting them:

import Lib;

transformation Trans(...) access Lib;

eclipse-qvt-oml-bot commented 4 days ago

By Toni Siljamäki on Dec 04, 2020 14:54

This is an industrial-scale and QVTO-based Model Compiler (MC) I began developing back in 2016,\ using the M2M and M2T approach EdW later explained in the TextM2M paper.\ This is not some toy-example transformation, it's about transforming ActorDSL models in PapyrusUML to code.

It has one main transformation with 80+ supporting library transformation files (~1.2 MB of QVTO code),\ in order to get a more manageable subject-matter-separation of transformation domain functions in separate files. ...and Yes: The libraries are imported where needed.

In this particular MC variant I have not yet included the transformation of Action Language for UML (like ALF),\ which currently has an additional 12 QVTO library files. (actually the reduced ALF = rALF, developed by IncQueryLabs in Budapest for Ericsson)\ In QVTO 3.6.0 there were no performance issues related to number of library files or local variables.

NOTE: A 25sec extra cost in parsing time is problematic for me, since it slows down both the while-editing-parsing time\ and the turnaround time for MC development iterations, which I need to be short.

As soon as you edit a file, the parser is restarted, so in worst case you have to stop editing for 25sec to get parsing feedback.\ See the note on this in attached slide #69.

If you by "inherit all local variables from all (transitively) imported libraries" refer to the "property" defined variables,\ I have only one such variable, which has a more global transformation time content, a variable for a created "object"\ which I'm passing around throughout the whole transformation, from start to end.

For such global transformation-time data a "static property object" variable would have been useful.

QUESTION 1: How can I enforce QVTO to automatically parse auto-opened QVTO files when a workspace is relaunched?\ This is my main problem right now! I don't appreciate having to close-and-reopen all QVTO files to enable their parsing.

QUESTION 2: Can I hack a QVTO Java file in some way and rebuild some plugin to make it happen, like in QVTO 3.6.0 ?

eclipse-qvt-oml-bot commented 4 days ago

By Christopher Gerking on Dec 08, 2020 07:54

(In reply to Toni Siljamäki from comment #12)

...and Yes: The libraries are imported where needed.

Like I said, try to import these libraries using the 'access' keyword.

If you by "inherit all local variables from all (transitively) imported libraries" refer to the "property" defined variables, I have only one such variable

The thing is: the QVTo compiler can't know that you have only one such variable. So it has to search all the imported libraries for potential conflicting variables. Although there are no conflicts, this takes a lot of time. Using the aforementioned 'access' mechanism, you could possibly reduce the search space and speed up the compilation.

QUESTION 1: How can I enforce QVTO to automatically parse auto-opened QVTO files when a workspace is relaunched?

Can't reproduce. For me all open QVTo files are parsed immediately on startup. Do you have auto-building turned on? Project->Build Automatically

Anyway, if this is a blocker for you, please file a separate bugzilla and provide a working example for reproduction.

eclipse-qvt-oml-bot commented 4 days ago

By Ed Willink on Dec 08, 2020 10:04

(In reply to Christopher Gerking from comment #13)

The thing is: the QVTo compiler can't know that you have only one such variable. So it has to search all the imported libraries

I've not understood this discussion, but this comment seems like a clear language/tool bug/limitation that should be fixed. Instinctively, rather than referring to "libVar" from an unknown source mandating an offensive search-the-world, you should be able to refer to "myLib::libVar" requiring just a precise load.

Can you please provide a simple example of this semantic concern so that I can investigate?

eclipse-qvt-oml-bot commented 4 days ago

By Christopher Gerking on Dec 08, 2020 12:22

Created attachment 284995 Example project demonstrating QVTo's two import mechanisms

(In reply to Ed Willink from comment #14)

I've not understood this discussion, but this comment seems like a clear language/tool bug/limitation that should be fixed. The "extends" keyword of QVTo implies default extension semantics, which are transitive just as in Java:

class A extends B: All public members of B are visible inside A.

class A extends X extends extends B: The public members of B are still visible inside A.

In particular, the "extends" keyword enables operations to be overridden, even transitively.

In contrast, using the "access" keyword only makes the members of the imported library accessible. Behind the scenes, a singleton instance of the library is created, which is registered as an implicit variable. So the members of that instance can be accessed, but they can't be overridden.

The actual problem is that Eclipse QVTo has always used "extends" as its default import mechanism whenever no explicit mechanism is indicated (see bug 468367). IMHO this isn't a good choice, which also violates the latest QVT specification. But of course a change would cause regressions.

Can you please provide a simple example of this semantic concern so that I can investigate? Example project attached.

:compression: qvto-import-mechanisms.zip