Bug 251277 - [plan] [ide] AsmManager should be non-singleton
Summary: [plan] [ide] AsmManager should be non-singleton
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: DEVELOPMENT   Edit
Hardware: PC Windows XP
: P2 major (vote)
Target Milestone: 1.6.3   Edit
Assignee: aspectj inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-10-17 16:05 EDT by Andrew Clement CLA
Modified: 2008-10-22 17:24 EDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Clement CLA 2008-10-17 16:05:06 EDT
With the use of AspectJ in IDEs like eclipse where multiple AspectJ projects co-exist and build independently, it is awkward that AsmManager is a singleton as we have to keep switching to the 'right model' before interacting with it or triggering a build.  Really there should be one structure model per project.  We already manage a compiler instance and associated state on a per-project basis.  The structure model should be anchored in this state.
Comment 1 Andrew Clement CLA 2008-10-17 16:07:36 EDT
Without supporting this it can be tricky for AJDT to do clever stuff like manipulate a projects model on a different thread as a build may start at any time and 'switch' the singleton to the project it is building.

The first step is to remove getDefault() in AsmManager and then pick up the debris from that explosion...

I now have it successfully compiling but some tests are failing.  Some areas, like the original Ajde code, really want to treat it as a singleton.  And many many many of the tests force a build then grab the singleton through the AsmManager.getDefault() rather than asking for the model for what just got built.
Comment 2 Andrew Clement CLA 2008-10-19 14:33:30 EDT
OK.  All tests passing and all AJDT core tests passing too (with changes in AJDT to not switch models).

Changes:
- removed getDefault from AsmManager
- introduced a static 'lastActiveStructureModel' field into AsmManager PURELY FOR TESTS that can't easily currently get the model attached to the compiler they've just used.  This is for testing only and is only stored if the AsmManager.recordingLastActiveStructureModel field is set to true.  AJDT switches that to false so we don't have any unnecessary model anchored in static.
- ProgramElements can now access the model in which they are a node.  Since the handle creation depends upon the model, this is OK.  it adds 4bytes to each programelement but that should not be an issue given what we are saving by not having two copies of the entire thing (ajdt's copy and our copy).
- AspectJElementHierarchy can also access the model for which it is the hierarchy.  It doesn't get serialized and since some tests check model write/read, it has to be reset on the hierarchy after reading in.
- FullPathHandleProvider deleted. 
- Removed silly 'dependsOnLocation()' method in HandleProvider interface, you dont want to ever use one that does dependsOnLocation
- AjState now holds an AsmManager rather than the pieces (hierarchy and relationshipmap)
- World now holds an AsmManager and that gets used by any weaving things that need a model
- Structure tests in ajde.core were the messiest as it really wanted to treat it as a singleton.  It can be forced to behave in such a way by setting 'AsmManager.forceSingletonBehaviour' but only useful for testing
- ajdoc is the worst problem.  its' tests are passing but I'm not convinced it is working 100% correctly

Still need to rename AsmManager to StructureModel but not doing that in initial commit
Comment 3 Andrew Eisenberg CLA 2008-10-19 16:20:50 EDT
Great work, Andy.  Looking forward to trying this out.
Comment 4 Andrew Clement CLA 2008-10-22 17:24:49 EDT
complete and committed into AJDT.  The ajdoc area could do with looking at but I'll put that off until it proves to be a problem.