Description David Slubicki 2006-07-13 09:32:46 EDT

Background
     ModelManagerImpl, a singleton, provides access to models via its various "get*Model*" methods, all of which are synchronized.  I understand the need to synchronize the retrieving/releasing of models, given that ModelManagerImpl manages the lifecycle of the shared models (keeping reference counts, etc).  The problem with the implementation (as I see it) is that most of the "get*Model*" methods will actually create the requested model if one does not exist, and creating a model is, potentially, very time consuming.  Various background threads (builders, validators, indexers, etc) competing with the main thread for access to ModelManagerImpl becomes, in certain scenarios, a performance bottleneck.
     
Potential Fix
     With limited knowledge of ModelManagerImpl, I just want to put this idea out there to see what is thought about its feasibility.
     My thought is that we could introduce an internal,  light-weight "model proxy" class (ModelProxy), instances of which would represent structured models.  ModelManagerImpl would synchronize solely on retrieving/releasing ModelProxy instances (which should be very fast) as opposed to instantiating and managing the actual IStructuredModel objects.  An individual ModelProxy instance would then provide synchronized access to its underlying model.  The first attempt to acquire the model from the proxy would result in the model being created.  As only the individual proxy instance associated with a specific model would be locked (via synchronization) during the creation of the underlying model, other threads attempting to acquire different models would not be blocked.  The proposed changes would be completely internal to ModelManagerImpl, with no API changes.
     

Test Implementation
     To test this idea, I decided to create a prototype (and I emphasize "prototype") by modifying the existing ModelManagerImpl.  This turned out to be far more work than I anticipated (the original class is around 2000 lines).  To manage the volume of changes I had to make, I first created an abstract inner class, ModelProxy, which defines the synchronized #getModel() method.  For each relevant public synchronized method I tried to apply the following pattern:
     - remove "synchronized" from the signature
     - create a corresponding private synchronized method with a return type of ModelProxy.   This method has the same name as the public method, with "_Proxy" appended to the end.  (note:  in some cases, a common "_Proxy"  method is used)
     - create a concrete subclass of ModelProxy for this method, overriding #getModel().  To this implementation of #getModel() I moved the existing logic for creating a model via this ModelManagerImpl method.
note:  I'm not suggesting we use this implementation - I wrote it solely to demonstrate the design.


Example Code
Logically, this is what model access currently looks like.  I condensed several methods down to one for clarity (note that it is synchronized):

public synchronized IStructuredModel getModelForEdit(IFile iFile) {
     String id = calculateId(iFile);
     IStructuredModel model= getModel(id);
     if (model == null) {
          model = createModel(iFile, EDIT);
          fManagedModels.put(id, model);
     } else
          _incrCount(model, EDIT);
}



Redesigned, this method would be broken into two  - looking something like this (note that only the private method is synchronized):

public getModelForEdit(IFile iFile) {
     IStructuredModel model = null;
     ModelProxy proxy = getModel_Proxy(iFile, EDIT);
     if (proxy != null)
          model = proxy.getModel();
     return model;
}

private synchronized ModelProxy getModel_Proxy(IFile iFile, ReadEditType rwType) {
     String id = calculateId(iFile);
     ModelProxy proxy= getModelProxy(id); //retrieves it from the dictionary
     if (proxy == null) {
          proxy = new ModelProxyByFile(id, iFile, rwType);
          fManagedProxies.put(id, proxy);
     } else
          _incrCount(proxy, rwType);

     return proxy;
}


The ModelProxyForFile class would look something like this:

ModelProxyByFile extends ModelProxy {
     ModelProxyByFile(String id, IFile file, ReadEditType rwType) {
          super(id, rwType);   // super will increment the reference count 
          ...  // store the file 
     }
     
     synchronized IStructuredModel getModel() {
          if (fModel == null) {
               // create the model
               // clear any data (passed in to the constructor) used in creating the model
          }                
          return fModel;
     }
}


I'll attach my prototype implementation, though I'm sure there must be many problems with it.  Also, I didn't know exactly what would be required to add this same sort of support for 
methods used to access/create IStructuredDocument objects, but I attempted to use a similar design.  A very small change was also needed in AbstractStructuredModel, which I'll attach.