From b4e0833b1b180f40b132e60ee49d897d409bc000 Mon Sep 17 00:00:00 2001 From: Abyss777 Date: Thu, 21 Jul 2016 09:56:41 +0500 Subject: Migrate from ReentrantReadWriteLocks to ConcurrentHashMaps in DeviceManager UpdateCahces incrementally --- src/org/traccar/database/DeviceManager.java | 286 +++++++++++----------------- 1 file changed, 106 insertions(+), 180 deletions(-) (limited to 'src/org/traccar') diff --git a/src/org/traccar/database/DeviceManager.java b/src/org/traccar/database/DeviceManager.java index 43d83d078..916e7fe67 100644 --- a/src/org/traccar/database/DeviceManager.java +++ b/src/org/traccar/database/DeviceManager.java @@ -18,15 +18,13 @@ package org.traccar.database; import java.sql.SQLException; import java.util.ArrayList; import java.util.Collection; -import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.locks.ReadWriteLock; -import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.concurrent.atomic.AtomicLong; import org.traccar.Config; import org.traccar.Context; @@ -43,14 +41,12 @@ public class DeviceManager implements IdentityManager { private final DataManager dataManager; private final long dataRefreshDelay; - private final ReadWriteLock devicesLock = new ReentrantReadWriteLock(); - private final Map devicesById = new HashMap<>(); - private final Map devicesByUniqueId = new HashMap<>(); - private long devicesLastUpdate; + private Map devicesById; + private Map devicesByUniqueId; + private AtomicLong devicesLastUpdate = new AtomicLong(0); - private final ReadWriteLock groupsLock = new ReentrantReadWriteLock(); - private final Map groupsById = new HashMap<>(); - private long groupsLastUpdate; + private Map groupsById; + private AtomicLong groupsLastUpdate = new AtomicLong(0); private final Map positions = new ConcurrentHashMap<>(); @@ -72,77 +68,76 @@ public class DeviceManager implements IdentityManager { } private void updateDeviceCache(boolean force) throws SQLException { - boolean needWrite; - devicesLock.readLock().lock(); - try { - needWrite = force || System.currentTimeMillis() - devicesLastUpdate > dataRefreshDelay; - } finally { - devicesLock.readLock().unlock(); - } - + boolean needWrite = force || System.currentTimeMillis() - devicesLastUpdate.get() > dataRefreshDelay; if (needWrite) { - devicesLock.writeLock().lock(); - try { - if (force || System.currentTimeMillis() - devicesLastUpdate > dataRefreshDelay) { - devicesById.clear(); - devicesByUniqueId.clear(); - GeofenceManager geofenceManager = Context.getGeofenceManager(); - for (Device device : dataManager.getAllDevices()) { - devicesById.put(device.getId(), device); - devicesByUniqueId.put(device.getUniqueId(), device); - if (geofenceManager != null) { - Position lastPosition = getLastPosition(device.getId()); - if (lastPosition != null) { - device.setGeofenceIds(geofenceManager.getCurrentDeviceGeofences(lastPosition)); - } + GeofenceManager geofenceManager = Context.getGeofenceManager(); + Collection databaseDevices = dataManager.getAllDevices(); + if (devicesById == null) { + devicesById = new ConcurrentHashMap<>(databaseDevices.size()); + } + if (devicesByUniqueId == null) { + devicesByUniqueId = new ConcurrentHashMap<>(databaseDevices.size()); + } + Set databaseDevicesIds = new HashSet<>(); + Set databaseDevicesUniqIds = new HashSet<>(); + for (Device device : databaseDevices) { + databaseDevicesIds.add(device.getId()); + databaseDevicesUniqIds.add(device.getUniqueId()); + if (devicesById.containsKey(device.getId())) { + Device cachedDevice = devicesById.get(device.getId()); + cachedDevice.setName(device.getName()); + cachedDevice.setGroupId(device.getGroupId()); + cachedDevice.setAttributes(device.getAttributes()); + if (!device.getUniqueId().equals(cachedDevice.getUniqueId())) { + devicesByUniqueId.remove(cachedDevice.getUniqueId()); + devicesByUniqueId.put(device.getUniqueId(), cachedDevice); + } + cachedDevice.setUniqueId(device.getUniqueId()); + } else { + devicesById.put(device.getId(), device); + devicesByUniqueId.put(device.getUniqueId(), device); + if (geofenceManager != null) { + Position lastPosition = getLastPosition(device.getId()); + if (lastPosition != null) { + device.setGeofenceIds(geofenceManager.getCurrentDeviceGeofences(lastPosition)); } } - devicesLastUpdate = System.currentTimeMillis(); + device.setStatus(Device.STATUS_OFFLINE); + device.setMotion(Device.STATUS_STOPPED); } - } finally { - devicesLock.writeLock().unlock(); } + for (Long cachedDeviceId : devicesById.keySet()) { + if (!databaseDevicesIds.contains(cachedDeviceId)) { + devicesById.remove(cachedDeviceId); + } + } + for (String cachedDeviceUniqId : devicesByUniqueId.keySet()) { + if (!databaseDevicesUniqIds.contains(cachedDeviceUniqId)) { + devicesByUniqueId.remove(cachedDeviceUniqId); + } + } + databaseDevicesIds.clear(); + databaseDevicesUniqIds.clear(); + devicesLastUpdate.set(System.currentTimeMillis()); } } @Override public Device getDeviceById(long id) { - devicesLock.readLock().lock(); - try { - return devicesById.get(id); - } finally { - devicesLock.readLock().unlock(); - } + return devicesById.get(id); } @Override public Device getDeviceByUniqueId(String uniqueId) throws SQLException { - boolean forceUpdate; - devicesLock.readLock().lock(); - try { - forceUpdate = !devicesByUniqueId.containsKey(uniqueId) && !config.getBoolean("database.ignoreUnknown"); - } finally { - devicesLock.readLock().unlock(); - } + boolean forceUpdate = !devicesByUniqueId.containsKey(uniqueId) && !config.getBoolean("database.ignoreUnknown"); updateDeviceCache(forceUpdate); - devicesLock.readLock().lock(); - try { - return devicesByUniqueId.get(uniqueId); - } finally { - devicesLock.readLock().unlock(); - } + return devicesByUniqueId.get(uniqueId); } public Collection getAllDevices() { - boolean forceUpdate; - devicesLock.readLock().lock(); - try { - forceUpdate = devicesById.isEmpty(); - } finally { - devicesLock.readLock().unlock(); - } + boolean forceUpdate = devicesById.isEmpty(); try { updateDeviceCache(forceUpdate); @@ -150,23 +145,13 @@ public class DeviceManager implements IdentityManager { Log.warning(e); } - devicesLock.readLock().lock(); - try { - return devicesById.values(); - } finally { - devicesLock.readLock().unlock(); - } + return devicesById.values(); } public Collection getDevices(long userId) throws SQLException { Collection devices = new ArrayList<>(); - devicesLock.readLock().lock(); - try { - for (long id : Context.getPermissionsManager().getDevicePermissions(userId)) { - devices.add(devicesById.get(id)); - } - } finally { - devicesLock.readLock().unlock(); + for (long id : Context.getPermissionsManager().getDevicePermissions(userId)) { + devices.add(devicesById.get(id)); } return devices; } @@ -174,56 +159,34 @@ public class DeviceManager implements IdentityManager { public void addDevice(Device device) throws SQLException { dataManager.addDevice(device); - devicesLock.writeLock().lock(); - try { - devicesById.put(device.getId(), device); - devicesByUniqueId.put(device.getUniqueId(), device); - } finally { - devicesLock.writeLock().unlock(); - } + devicesById.put(device.getId(), device); + devicesByUniqueId.put(device.getUniqueId(), device); } public void updateDevice(Device device) throws SQLException { dataManager.updateDevice(device); - devicesLock.writeLock().lock(); - try { - devicesById.put(device.getId(), device); - devicesByUniqueId.put(device.getUniqueId(), device); - } finally { - devicesLock.writeLock().unlock(); - } + devicesById.put(device.getId(), device); + devicesByUniqueId.put(device.getUniqueId(), device); } public void updateDeviceStatus(Device device) throws SQLException { dataManager.updateDeviceStatus(device); - - devicesLock.writeLock().lock(); - try { - if (devicesById.containsKey(device.getId())) { - Device cachedDevice = devicesById.get(device.getId()); - cachedDevice.setStatus(device.getStatus()); - cachedDevice.setMotion(device.getMotion()); - } - } finally { - devicesLock.writeLock().unlock(); + if (devicesById.containsKey(device.getId())) { + Device cachedDevice = devicesById.get(device.getId()); + cachedDevice.setStatus(device.getStatus()); + cachedDevice.setMotion(device.getMotion()); } } public void removeDevice(long deviceId) throws SQLException { dataManager.removeDevice(deviceId); - devicesLock.writeLock().lock(); - try { - if (devicesById.containsKey(deviceId)) { - String deviceUniqueId = devicesById.get(deviceId).getUniqueId(); - devicesById.remove(deviceId); - devicesByUniqueId.remove(deviceUniqueId); - } - } finally { - devicesLock.writeLock().unlock(); + if (devicesById.containsKey(deviceId)) { + String deviceUniqueId = devicesById.get(deviceId).getUniqueId(); + devicesById.remove(deviceId); + devicesByUniqueId.remove(deviceUniqueId); } - positions.remove(deviceId); } @@ -234,13 +197,8 @@ public class DeviceManager implements IdentityManager { dataManager.updateLatestPosition(position); - devicesLock.writeLock().lock(); - try { - if (devicesById.containsKey(position.getDeviceId())) { - devicesById.get(position.getDeviceId()).setPositionId(position.getId()); - } - } finally { - devicesLock.writeLock().unlock(); + if (devicesById.containsKey(position.getDeviceId())) { + devicesById.get(position.getDeviceId()).setPositionId(position.getId()); } positions.put(position.getDeviceId(), position); @@ -272,47 +230,40 @@ public class DeviceManager implements IdentityManager { } private void updateGroupCache(boolean force) throws SQLException { - boolean needWrite; - groupsLock.readLock().lock(); - try { - needWrite = force || System.currentTimeMillis() - groupsLastUpdate > dataRefreshDelay; - } finally { - groupsLock.readLock().unlock(); - } + boolean needWrite = force || System.currentTimeMillis() - groupsLastUpdate.get() > dataRefreshDelay; if (needWrite) { - groupsLock.writeLock().lock(); - try { - if (force || System.currentTimeMillis() - groupsLastUpdate > dataRefreshDelay) { - groupsById.clear(); - for (Group group : dataManager.getAllGroups()) { - groupsById.put(group.getId(), group); - } - groupsLastUpdate = System.currentTimeMillis(); + Collection databaseGroups = dataManager.getAllGroups(); + if (groupsById == null) { + groupsById = new ConcurrentHashMap<>(databaseGroups.size()); + } + Set databaseGroupsIds = new HashSet<>(); + for (Group group : databaseGroups) { + databaseGroupsIds.add(group.getId()); + if (groupsById.containsKey(group.getId())) { + Group cachedGroup = groupsById.get(group.getId()); + cachedGroup.setName(group.getName()); + cachedGroup.setGroupId(group.getGroupId()); + } else { + groupsById.put(group.getId(), group); + } + } + for (Long cachedGroupId : groupsById.keySet()) { + if (!databaseGroupsIds.contains(cachedGroupId)) { + devicesById.remove(cachedGroupId); } - } finally { - groupsLock.writeLock().unlock(); } + databaseGroupsIds.clear(); + groupsLastUpdate.set(System.currentTimeMillis()); } } public Group getGroupById(long id) { - groupsLock.readLock().lock(); - try { - return groupsById.get(id); - } finally { - groupsLock.readLock().unlock(); - } + return groupsById.get(id); } public Collection getAllGroups() { - boolean forceUpdate; - groupsLock.readLock().lock(); - try { - forceUpdate = groupsById.isEmpty(); - } finally { - groupsLock.readLock().unlock(); - } + boolean forceUpdate = groupsById.isEmpty(); try { updateGroupCache(forceUpdate); @@ -320,12 +271,7 @@ public class DeviceManager implements IdentityManager { Log.warning(e); } - groupsLock.readLock().lock(); - try { - return groupsById.values(); - } finally { - groupsLock.readLock().unlock(); - } + return groupsById.values(); } public Collection getGroups(long userId) throws SQLException { @@ -337,50 +283,30 @@ public class DeviceManager implements IdentityManager { } private void checkGroupCycles(Group group) { - groupsLock.readLock().lock(); - try { - Set groups = new HashSet<>(); - while (group != null) { - if (groups.contains(group.getId())) { - throw new IllegalArgumentException("Cycle in group hierarchy"); - } - groups.add(group.getId()); - group = groupsById.get(group.getGroupId()); + Set groups = new HashSet<>(); + while (group != null) { + if (groups.contains(group.getId())) { + throw new IllegalArgumentException("Cycle in group hierarchy"); } - } finally { - groupsLock.readLock().unlock(); + groups.add(group.getId()); + group = groupsById.get(group.getGroupId()); } } public void addGroup(Group group) throws SQLException { checkGroupCycles(group); dataManager.addGroup(group); - groupsLock.writeLock().lock(); - try { - groupsById.put(group.getId(), group); - } finally { - groupsLock.writeLock().unlock(); - } + groupsById.put(group.getId(), group); } public void updateGroup(Group group) throws SQLException { checkGroupCycles(group); dataManager.updateGroup(group); - groupsLock.writeLock().lock(); - try { - groupsById.put(group.getId(), group); - } finally { - groupsLock.writeLock().unlock(); - } + groupsById.put(group.getId(), group); } public void removeGroup(long groupId) throws SQLException { dataManager.removeGroup(groupId); - groupsLock.writeLock().lock(); - try { - groupsById.remove(groupId); - } finally { - groupsLock.writeLock().unlock(); - } + groupsById.remove(groupId); } } -- cgit v1.2.3 From 35eea73b2b9e381efd5da60662f0f64f59b91406 Mon Sep 17 00:00:00 2001 From: Abyss777 Date: Thu, 21 Jul 2016 17:39:04 +0500 Subject: Act with atomic more carefully --- src/org/traccar/database/DeviceManager.java | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) (limited to 'src/org/traccar') diff --git a/src/org/traccar/database/DeviceManager.java b/src/org/traccar/database/DeviceManager.java index 916e7fe67..f442dfb19 100644 --- a/src/org/traccar/database/DeviceManager.java +++ b/src/org/traccar/database/DeviceManager.java @@ -43,10 +43,10 @@ public class DeviceManager implements IdentityManager { private Map devicesById; private Map devicesByUniqueId; - private AtomicLong devicesLastUpdate = new AtomicLong(0); + private AtomicLong devicesLastUpdate = new AtomicLong(); private Map groupsById; - private AtomicLong groupsLastUpdate = new AtomicLong(0); + private AtomicLong groupsLastUpdate = new AtomicLong(); private final Map positions = new ConcurrentHashMap<>(); @@ -68,8 +68,10 @@ public class DeviceManager implements IdentityManager { } private void updateDeviceCache(boolean force) throws SQLException { - boolean needWrite = force || System.currentTimeMillis() - devicesLastUpdate.get() > dataRefreshDelay; - if (needWrite) { + + long lastUpdate = devicesLastUpdate.get(); + if (force || System.currentTimeMillis() - lastUpdate > dataRefreshDelay + && devicesLastUpdate.compareAndSet(lastUpdate, System.currentTimeMillis())) { GeofenceManager geofenceManager = Context.getGeofenceManager(); Collection databaseDevices = dataManager.getAllDevices(); if (devicesById == null) { @@ -79,10 +81,10 @@ public class DeviceManager implements IdentityManager { devicesByUniqueId = new ConcurrentHashMap<>(databaseDevices.size()); } Set databaseDevicesIds = new HashSet<>(); - Set databaseDevicesUniqIds = new HashSet<>(); + Set databaseDevicesUniqueIds = new HashSet<>(); for (Device device : databaseDevices) { databaseDevicesIds.add(device.getId()); - databaseDevicesUniqIds.add(device.getUniqueId()); + databaseDevicesUniqueIds.add(device.getUniqueId()); if (devicesById.containsKey(device.getId())) { Device cachedDevice = devicesById.get(device.getId()); cachedDevice.setName(device.getName()); @@ -112,13 +114,12 @@ public class DeviceManager implements IdentityManager { } } for (String cachedDeviceUniqId : devicesByUniqueId.keySet()) { - if (!databaseDevicesUniqIds.contains(cachedDeviceUniqId)) { + if (!databaseDevicesUniqueIds.contains(cachedDeviceUniqId)) { devicesByUniqueId.remove(cachedDeviceUniqId); } } databaseDevicesIds.clear(); - databaseDevicesUniqIds.clear(); - devicesLastUpdate.set(System.currentTimeMillis()); + databaseDevicesUniqueIds.clear(); } } @@ -230,9 +231,10 @@ public class DeviceManager implements IdentityManager { } private void updateGroupCache(boolean force) throws SQLException { - boolean needWrite = force || System.currentTimeMillis() - groupsLastUpdate.get() > dataRefreshDelay; - if (needWrite) { + long lastUpdate = groupsLastUpdate.get(); + if (force || System.currentTimeMillis() - lastUpdate > dataRefreshDelay + && groupsLastUpdate.compareAndSet(lastUpdate, System.currentTimeMillis())) { Collection databaseGroups = dataManager.getAllGroups(); if (groupsById == null) { groupsById = new ConcurrentHashMap<>(databaseGroups.size()); @@ -254,7 +256,6 @@ public class DeviceManager implements IdentityManager { } } databaseGroupsIds.clear(); - groupsLastUpdate.set(System.currentTimeMillis()); } } -- cgit v1.2.3 From e269c81b1b8044d051196e97f0884935c153feb0 Mon Sep 17 00:00:00 2001 From: Abyss777 Date: Thu, 21 Jul 2016 17:44:57 +0500 Subject: Added parentheses --- src/org/traccar/database/DeviceManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/org/traccar') diff --git a/src/org/traccar/database/DeviceManager.java b/src/org/traccar/database/DeviceManager.java index f442dfb19..4dd7b41cb 100644 --- a/src/org/traccar/database/DeviceManager.java +++ b/src/org/traccar/database/DeviceManager.java @@ -70,7 +70,7 @@ public class DeviceManager implements IdentityManager { private void updateDeviceCache(boolean force) throws SQLException { long lastUpdate = devicesLastUpdate.get(); - if (force || System.currentTimeMillis() - lastUpdate > dataRefreshDelay + if ((force || System.currentTimeMillis() - lastUpdate > dataRefreshDelay) && devicesLastUpdate.compareAndSet(lastUpdate, System.currentTimeMillis())) { GeofenceManager geofenceManager = Context.getGeofenceManager(); Collection databaseDevices = dataManager.getAllDevices(); @@ -233,7 +233,7 @@ public class DeviceManager implements IdentityManager { private void updateGroupCache(boolean force) throws SQLException { long lastUpdate = groupsLastUpdate.get(); - if (force || System.currentTimeMillis() - lastUpdate > dataRefreshDelay + if ((force || System.currentTimeMillis() - lastUpdate > dataRefreshDelay) && groupsLastUpdate.compareAndSet(lastUpdate, System.currentTimeMillis())) { Collection databaseGroups = dataManager.getAllGroups(); if (groupsById == null) { -- cgit v1.2.3