Opened 13 years ago
Closed 13 years ago
#7736 closed enhancement (wontfix)
[Patch] replaced synchronized integer access with AtomicInteger
| Reported by: | Locked | Owned by: | team |
|---|---|---|---|
| Priority: | normal | Milestone: | |
| Component: | Core | Version: | |
| Keywords: | Cc: |
Description
The patch concerns org.openstreetmap.gui.jmapviewer.JobDispatcher
- some integers where wrapped in synchronized blocks for atomic access. By applying this patch the ints (including the synchronized blocks) can be replaced by AtomicIntegers. The semantics is not changed yet the code should be more robust if anyone ever needs to modify the counters.
- in executeJobs() an Exception was declared to be caught even though it could not be thrown. By applying the patch the catch(Exception e) is removed.
Attachments (1)
Change History (7)
by , 13 years ago
| Attachment: | JobDispatcher.patch added |
|---|
follow-up: 3 comment:1 by , 13 years ago
Currently, we are in stabilization phase, which means that no code changes different from bug fixes are applied (see DevelopersGuide/Schedule). Afterwards, this might be of interest.
comment:2 by , 13 years ago
| Summary: | replaced synchronized integer access with AtomicInteger → [Patch] replaced synchronized integer access with AtomicInteger |
|---|
Also, please annotate tickets containing patches with a [Patch] prefix in the summary (or similar).
comment:3 by , 13 years ago
Replying to simon04:
Currently, we are in stabilization phase, which means that no code changes different from bug fixes are applied (see DevelopersGuide/Schedule). Afterwards, this might be of interest.
Thanks for the information.
It is totally okay. Just stumbled upon the class yesterday and tried to refactor it. I'd be glad if the patch finds it's way into Josm at some time.
comment:4 by , 13 years ago
Replying to Locked:
The patch concerns org.openstreetmap.gui.jmapviewer.JobDispatcher
- some integers where wrapped in synchronized blocks for atomic access. By applying this patch the ints (including the synchronized blocks) can be replaced by AtomicIntegers. The semantics is not changed yet the code should be more robust if anyone ever needs to modify the counters.
I see no real need for this change. If anyone makes major functional improvements to this class, they can switch to AtomicIntegers, if preferred.
- in executeJobs() an Exception was declared to be caught even though it could not be thrown. By applying the patch the catch(Exception e) is removed.
Why? It's a Runnable and RuntimeException, e.g. NPE, can be thrown inside the run() method. This would kill the thread and mess up job management.
comment:5 by , 13 years ago
I thought I had checked it twice but was thinking too much in ExecutorService - sorry.
patch for both changes