Bugzilla – Full Text Bug Listing
| Summary: | [api] AbstractTaskContainer classes should store children in ConcurrentHashMap | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Mik Kersten <mik.kersten> | ||||||
| Component: | Mylyn | Assignee: | Steffen Pingel <steffen.pingel> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||
| Severity: | major | ||||||||
| Priority: | P2 | CC: | robert.elves | ||||||
| Version: | unspecified | ||||||||
| Target Milestone: | 3.0 | ||||||||
| Hardware: | PC | ||||||||
| OS: | All | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Mik Kersten
As I already commented in bug 167402, you can actually use ConcurrentHashMap with single bogus instance as a value. Then map.keySet() will give you instance of the concurrent set. This is related to implementing a scheme for concurrent access to the task list and ensuring consistency of the task list data model: - The task list is a directed graph of linked AbstractTaskContainter objects. The improve performance the task list needs to guarantee that the graph has no cycles. All modifications to the data model have to be validated and synchronized. - The tasks framework needs to provide means to lock the task list for modifications and to retrieve consistent snapshots of the task list for saving and export. Steffen's comment from bug#213228: Another spot that needs to be reviewed is the TaskActiviationHistory. It stores tasks in a list and relies on equals() for removing tasks. Created attachment 97913 [details]
patch
Created attachment 97914 [details]
mylyn/context/zip
I have replaced the CopyOnWriteArraySet with a CopyOnWriteArrayList in AbstractTaskContainer and changed the signature of getChildren() to return a collection. Consistency of the task list data model is now validated in the TaskList class. Using a list instead of set (which uses a list as the backend) should yield a minimal performance gain since the list of children is now only for duplicates once rather instead of twice. I have attached a patch in case we need to revert this change. Steffen: there is a benefit to returning a collection because it is the most generic thing to return, but we should consider this decision when doing API review. We use sets in numerous places in order to make it clear to clients that they will not see duplicates and do not need to check for them, and that there is no promise made about ordering. Those are properties that could be worth retaining in the API. |