Architecture Council/Top Ten Recommendations

Introduction

This is a work in progress by the Eclipse Architecture Council to collect a "top ten" list of architectural recommendations for Eclipse developers. (Started in August 2008)

There is a similar top ten list from a UI Guidelines perspective (Top Ten Lists Working Page).

It is the nature of such a top ten list that it is not exhaustive but only highlights issues that tend to come up more frequently.

Eclipse Architecture Council members are invited to add items to the lists, or comment on existing items. We can use the monthly phone meetings to discuss relative priorities and what the final "top ten list" should look like. Please try to come up with fairly concrete items on which we can give good advice.

Top Ten

Top Ten Good Practices (work in progress)

• Minimize plug-in dependencies. You will end up with easier to maintain, evolve, and easier to reuse components if you find the right granularity. One good rule of thumb is to separate UI and non-UI code into different plug-ins. Another principle is to define layers along which you can split your plug-ins. For example, put custom widgets that only depend on SWT in one plug-in, use a separate plug-in for code that additionally requires JFace, and then another plug-in that additionally requires the Workbench APIs. MOB: Following this may mean getting lots of very small plug-ins. Can we give advice on what number of plug-ins is still healthy? Or what number of classes should be in a plug-in to make it worth living separately? WTB: What about importing packages instead of requiring bundles?
• Be asynchronous. Where possible, seek temporal decoupling between components. This means using asynchronous communication or events, and avoiding unnecessary dependencies or contractual promises about when particular actions or state changes will occur. In traditional software development the decision between using synchronous or asynchronous mechanisms is not so clear. Asynchronous APIs can be more difficult to program and use, and developers often favour simpler synchronous APIs. However, in the highly extensible and modular world of Eclipse development, asynchronous mechanisms are much more compelling. In particular, locking is highly deadlock prone because any interaction with another bundle can result in arbitrary third party code being called which may in turn acquire its own locks. Writing more asynchronous code also maximizes opportunities for concurrency, which avoids blocking the end user, and allows for better exploitation of the capabilities of modern multi-core hardware.
• Don't assume your bundle is the center of the world. When developing a bundle it is easy to adopt the mindset that your bundle is terribly important and must always be visible/loaded/available. When developing software in Eclipse it is important to think about how your bundle fits into applications where thousands of other bundles are present. This means imposing minimal or no overhead on memory/CPU/screen real estate until your bundle is concretely needed by the user or application. In Eclipse the user and/or the application are boss, and all other bundles are just along for the ride. In other words, don't speak until spoken to.
• Be lazy. (probably a duplicate of above, but perhaps "be lazy" is a better name)
• Long-running operations should report progress and be cancelable. MOB: Add hyperlinks into how to report progress from Jobs, and how to use SubProgressMonitor properly -- it's sometimes unclear when beginTask() / subTask() / done() should actually be used, and how to best split some work into subitems - especially when asynchronous APIs are used.
• Think API. When designing your application, think about work items not only be called from your own UI, but also from other programs. Design your code around the API that's needed for third parties to call your code. Strive to keep your exposed API minimal. Document every aspect of your API in Javadoc, including side conditions such as threading aspects, state of your model, and range of valid input. Your API doesn't need to be exposed as public backward-compatible interfaces from the first iteration on: keeping your API "internal" but documented is already valuable See also: API First article, and the Wiki Eclipse/API Central.
• Create Unittests early. At the time you create some new API or functionality, you have the best insight in how it's supposed to work. Create Unittests as early as possible, and you'll not only find bugs in your code, weaknesses in your documentation and dependencies, but also gain safety in future changes and refactorings. On the AspectJ project we adopted a process for dealing with bug reports that always started with reproducing the bug in a failing test case in the test suite. Then we would fix the issue, add any other tests as needed, and commit. This way we ensured that we never allowed that same bug back into the compiler in the future. This practice has proved absolutely essentially to ensuring the quality of AspectJ over several major overhauls and avoiding regressions.
• Producer/Consumer. somewhat of a duplicate of other comments but I find this spin quite powerful Always recognize that there are function producers and function consumers. Each has a different context. For example, bundles that implement actions should not situate them in the UI. Always know which you are when discussing, designing, implementing, configuring.
• Separate policy and mechanism. As the producer of some mechanism, seek to minimize the assumptions you make about the policies that will drive the mechanism. Consumers will define the policy to suit their use-case. For example, write a generic mechanism that polls a filesystem location some number of times at some frequency and does some action all according to a policy rather than have N implementations of specific mechanisms each encapsulating the policy. This enables your function to be used in a wider range of situations with greater ease.
• Keep simple things simple. Make hard things possible. It is great to componentize and create frameworks and platforms. These make hard things possible. Without additional steps to smooth the 80% case, everyone has to deal with maximal complexity and the simple things are not simple.
• Package coherence. As you define your Java packages, pay particular attention to putting only related things together. Putting too much in one package inhibits future refactoring as packages should not be split across bundles. A concrete example from the early days of Eclipse... In the runtime we had the package org.eclipse.core.runtime that contained plugin stuff, extension registry stuff, and a mess of helpers. With the introduction of Equinox we refactored the bundles and wanted to move the registry into its own bundle. Unfortunately, this caused us to have org.eclipse.core.runtime in multiple places. This in turn inhibits people using another best practice -- use Import-Package rather than Require-Bundle.
• Be aware of the deployment context of your bundle. If you are developing a runtime or non-UI component that could be deployed to a server environment (such as OSGi on webSphere), be aware that some of the assumptions for a desktop app doesn't apply. For example, you do not want to have dependcy on a UI bundle direct or indirectly. You can not assume you can write to file system in the workspace. You might want to avoid create your own threads or using events that upsets the central thread or queue mgmt of the app server. You might want to avoid any locking/syncronization that could impact the scaleability of your component in a high concurrency application.

Top Ten Bad Practices (work in progress)

• Don't acquire locks or do interesting work in the UI thread. The responsiveness of an Eclipse-based application will be negatively affected by this. As a rule of thumb, anything that takes longer than 200ms is noticeable as a delay by end-users and thus should not be done on the UI thread where it will block the UI. Code that acquires a lock does not belong on the UI thread because this may block the UI for an indefinite amount of time. It can also easily lead to deadlock situations, or other dangerous side-effects such as a dialog that pops up telling the user that their current operation is waiting on a background thread. This dialog is dangerous because it spins the event loop, and thus potentially runs asyncExec code that can alter the state of the operation that is waiting on a lock. MOB: Can we add hyperlinks into Docs or Articles that tell how to spawn background jobs from a UI action, collect the results, report progress, and modify the UI as a result?
• Don't spin secondary event loops. The Display.readAndDispatch() call should be reserved for the Framework only. Fork off a Job for long-running work, or consider asynchronous callbacks instead of spinning the event loop.
• Don't copy&paste any code without Copyright Headers and Provenience Info. You might be inclined top copy&paste some code just to try something out, but might then find that functionality worthwile. Copying code without its copyright header typically violates the license, so better keep the Copyright with the code right away and add some notice (hyperlink) where you got the stuff from.

Other

Design Patterns

• Eclipse Design Patterns:

1. JFace Viewer - Implementation of Pluggable Adapter Pattern
2. JFace Viewer - Implementation of Strategy Pattern

• See this EclipseCon 2009 Tutorial

Anti-patterns

• Plug-in Development Anti-patterns
• See this EclipseCon 2009 Tutorial

Concurrency Do's and Don'ts

This section is initially provided by a team of Software Architects from SAP AG. On development of SAP Netweaver Developer Studio we faced a lot of concurrency issues. This document contains the Do's and Don'ts we derived from the deadlock issues. Please help to improve the document by adding the Do's and Don'ts that you have identified from your experiences with Concurrency in Eclipse.

Do not acquire resource locks in the UI thread

Deadlocks can occur if resource locks are acquired in the UI thread because there are background threads that hold resource locks and must go to the UI thread via Display.syncExec.
Example: execute delete on a resource in Eclipse Navigator. Navigator starts a job to delete the resource. The job calls resource.delete. Inside resource.delete a connected source repository comes into the game and needs to send a logon or checkout dialog to the user. A deadlock occurs if the UI thread waits for a conflicting resource at the same time.

Unclear policy in Eclipse

To prevent deadlocks on resources one of the following rules must apply for all Eclipse plug-ins:

• Rule 1: It is allowed to call Display.syncExec holding a resource lock and it is forbidden to acquire resource locks in the UI thread.
• Rule 2: It is allowed to acquire resource locks in the UI thread and it is forbidden to call Display.syncExec when holding a resource lock. Since we do not want to block the UI thread only rule 1 can apply.  But Eclipse itself acquires resource locks in the UI thread e.g. when JDT is saving the Java editor.

Locks in the UI thread in general

Another question is if all kinds of locks are forbidden in the UI thread. If this would be the case it would not be possible to read data from a thread safe model in the UI thread and pass the data to widgets. Perhaps the rule should be softened in the way that it is allowed to acquire locks in the UI thread that are blocked by other threads only for a short time.

Do not transfer resource locks between threads

We saw that the Eclipse refactoring framework locks the Workspace and transfers the rule to another thread.

Do not switch to ModalContext in UI update code

UI update code like a children provider of a tree should not switch to ModalContext to retrieve data. Such a switch can easily produce a dead lock:  

• Thread Worker 1 acquires a resource lock and switches to the UI thread via Display.syncExec
• The UI thread switches to ModalContext and ModalContext waits for a conflicting resource lock

Always acquire locks in the same order

Different kinds of locks should always be acquired in the same order to avoid deadlocks. Eclipse resource locks must be the first kind of lock to be acquired. Keep in mind that the UI thread is also a kind of lock (UI Lock). The UI thread implicitly owns the UI lock. Calling Display.syncExec tries to acquire the UI Lock.

Avoid to wait for completion of other threads

Example: a thread T schedules a job J and waits for the job result (join). This implies that T also waits for the locks of J.

Avoid calling Display.syncExec when holding a lock that can occur in the UI thread

Since there are currently components that acquire resource locks in the UI thread or wait indirectly on resource locks in the UI thread via ModalContext it is always a risk to call Display.syncExec when holding a resource lock.

Do not aquire locks in event listeners if you do not know the context

Deadlocks can occur if the caller of the listener holds locks. Do the work asynchronously in a job instead.

Document the locking behavior of your methods

The Javadoc of a public method should describe the locking behavior of the method.