Skip to content

Commit

Permalink
Merge pull request #1604 from garvankeeley/optimize-observation-point…
Browse files Browse the repository at this point in the history 
…-thread-access-during-draw

Remove observation point list copy during draw operations
  • Loading branch information
@crankycoder
crankycoder committed Apr 6, 2015
2 parents aaf503a + f155098 commit cb9034c
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 80 deletions.
27 changes: 10 additions & 17 deletions android/src/main/java/org/mozilla/mozstumbler/client/ObservedLocationsReceiver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@

import java.lang.ref.WeakReference;
import java.util.Collections;

import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
Expand All @@ -41,7 +40,7 @@ public class ObservedLocationsReceiver extends BroadcastReceiver {
private static final int MAX_QUEUED_MLS_POINTS_TO_FETCH = 10;
private static final long FREQ_FETCH_MLS_MS = 5 * 1000;
private static ObservedLocationsReceiver sInstance;
private final LinkedList<ObservationPoint> mCollectionPoints = new LinkedList<ObservationPoint>();
private final List<ObservationPoint> mCollectionPoints = Collections.synchronizedList(new LinkedList<ObservationPoint>());
private final List<ObservationPoint> mQueuedForMLS =
Collections.synchronizedList(new LinkedList<ObservationPoint>());
private final Handler mHandler = new Handler(Looper.getMainLooper());
Expand All @@ -67,13 +66,9 @@ public static ObservedLocationsReceiver getInstance() {
return sInstance;
}

/*
This returns a copy of the observation points so that we don't care about concurrency.
*/
public synchronized LinkedList<ObservationPoint> getObservationPoints() {
// Return a copy so that we don't need to care about locking
return new LinkedList<ObservationPoint>(mCollectionPoints);
}
public synchronized List<ObservationPoint> getObservationPoints_callerMustLock() {
return mCollectionPoints;
}

// Must call when map activity stopped, to clear the reference to the map activity object
public void removeMapActivity() {
Expand Down Expand Up @@ -164,13 +159,13 @@ public void onReceive(Context context, Intent intent) {

synchronized(mCollectionPoints) {
while (mCollectionPoints.size() > MAX_SIZE_OF_POINT_LISTS) {
mCollectionPoints.removeFirst();
mCollectionPoints.remove(0);
}

if (mCollectionPoints.size() > 0) {
observation.mHeading = observation.pointGPS.bearingTo(mCollectionPoints.getLast().pointGPS);
observation.mHeading = observation.pointGPS.bearingTo(mCollectionPoints.get(mCollectionPoints.size() - 1).pointGPS);
}
mCollectionPoints.addLast(observation);
mCollectionPoints.add(observation);
}

if (getMapActivity() == null) {
Expand All @@ -190,10 +185,8 @@ private void addObservationPointToMap() {
return;
}

synchronized(mCollectionPoints) {
getMapActivity().newObservationPoint(mCollectionPoints.getLast());
synchronized (mCollectionPoints) {
getMapActivity().newObservationPoint(mCollectionPoints.get(mCollectionPoints.size() - 1));
}
}


}
}
129 changes: 67 additions & 62 deletions android/src/main/java/org/mozilla/mozstumbler/client/mapview/ObservationPointsOverlay.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.ListIterator;

class ObservationPointsOverlay extends Overlay {
Expand Down Expand Up @@ -122,15 +123,17 @@ public void zoomChanged(MapView mapView) {
mHashedGrid = new LinkedHashMap<Integer, ObservationPoint>();
mHashedGridAnchorPoint = new Point(mapView.getScrollX(), mapView.getScrollY());
final Projection pj = mapView.getProjection();
LinkedList<ObservationPoint> points = ObservedLocationsReceiver.getInstance().getObservationPoints();
final Iterator<ObservationPoint> i = points.iterator();
final Point gps = new Point();
ObservationPoint point;
Point zero = new Point(0, 0);
while (i.hasNext()) {
point = i.next();
pj.toPixels(point.pointGPS, gps);
addToGridHash(point, gps, zero);
List<ObservationPoint> points = ObservedLocationsReceiver.getInstance().getObservationPoints_callerMustLock();
synchronized (points) {
final Iterator<ObservationPoint> i = points.iterator();
final Point gps = new Point();
ObservationPoint point;
Point zero = new Point(0, 0);
while (i.hasNext()) {
point = i.next();
pj.toPixels(point.pointGPS, gps);
addToGridHash(point, gps, zero);
}
}
}

Expand All @@ -148,72 +151,74 @@ private int toTypeBitField(ObservationPoint point) {

protected void draw(Canvas c, MapView osmv, boolean shadow) {
final long endTime = SystemClock.uptimeMillis() + DRAW_TIME_MILLIS;
LinkedList<ObservationPoint> points = ObservedLocationsReceiver.getInstance().getObservationPoints();
if (shadow || points.size() < 1) {
return;
}
List<ObservationPoint> points = ObservedLocationsReceiver.getInstance().getObservationPoints_callerMustLock();
synchronized (points) {
if (shadow || points.size() < 1) {
return;
}

final Projection pj = osmv.getProjection();
final float radiusInnerRing = mSize3px;
final Projection pj = osmv.getProjection();
final float radiusInnerRing = mSize3px;

int count = 0;
// The overlay occupies the entire screen, so this returns the screen (0,0,w,h).
Rect clip = c.getClipBounds();
int count = 0;
// The overlay occupies the entire screen, so this returns the screen (0,0,w,h).
Rect clip = c.getClipBounds();

if (mHashedGrid == null || mHashedGrid.size() < 1) {
return;
}
if (mHashedGrid == null || mHashedGrid.size() < 1) {
return;
}

final Point gps = new Point();
final Point gps = new Point();

ArrayList<HashMap.Entry<Integer, ObservationPoint>> arrayList =
new ArrayList<HashMap.Entry<Integer, ObservationPoint>>(mHashedGrid.entrySet());
ListIterator<HashMap.Entry<Integer, ObservationPoint>> revIterator =
arrayList.listIterator(mHashedGrid.size());
while (revIterator.hasPrevious()) {
HashMap.Entry<Integer, ObservationPoint> entry = revIterator.previous();
ObservationPoint point = entry.getValue();
pj.toPixels(point.pointGPS, gps);
ArrayList<HashMap.Entry<Integer, ObservationPoint>> arrayList =
new ArrayList<HashMap.Entry<Integer, ObservationPoint>>(mHashedGrid.entrySet());
ListIterator<HashMap.Entry<Integer, ObservationPoint>> revIterator =
arrayList.listIterator(mHashedGrid.size());
while (revIterator.hasPrevious()) {
HashMap.Entry<Integer, ObservationPoint> entry = revIterator.previous();
ObservationPoint point = entry.getValue();
pj.toPixels(point.pointGPS, gps);

if (!clip.contains(gps.x, gps.y)) {
continue;
}
if (!clip.contains(gps.x, gps.y)) {
continue;
}

boolean hasWifiScan = point.mWifiCount > 0;
boolean hasCellScan = point.mCellCount > 0;
boolean hasWifiScan = point.mWifiCount > 0;
boolean hasCellScan = point.mCellCount > 0;

if (hasWifiScan && !hasCellScan) {
drawWifiScan(c, gps);
} else if (hasCellScan && !hasWifiScan) {
drawCellScan(c, gps);
} else {
drawDot(c, gps, radiusInnerRing, mGreenPaint, mBlackStrokePaint);
}
if (hasWifiScan && !hasCellScan) {
drawWifiScan(c, gps);
} else if (hasCellScan && !hasWifiScan) {
drawCellScan(c, gps);
} else {
drawDot(c, gps, radiusInnerRing, mGreenPaint, mBlackStrokePaint);
}

if ((++count % TIME_CHECK_MULTIPLE == 0) && (SystemClock.uptimeMillis() > endTime)) {
break;
if ((++count % TIME_CHECK_MULTIPLE == 0) && (SystemClock.uptimeMillis() > endTime)) {
break;
}
}
}

if (!mOnMapShowMLS) {
return;
}

// Draw as a 2nd layer over the observation points
final Point mls = new Point();
revIterator = arrayList.listIterator(mHashedGrid.size());
while (revIterator.hasPrevious()) {
HashMap.Entry<Integer, ObservationPoint> entry = revIterator.previous();
ObservationPoint point = entry.getValue();
if (point.pointMLS != null) {
pj.toPixels(point.pointGPS, gps);
pj.toPixels(point.pointMLS, mls);
drawDot(c, mls, radiusInnerRing - 1, mRedPaint, mBlackStrokePaintThin);
c.drawLine(gps.x, gps.y, mls.x, mls.y, mBlackMLSLinePaint);
if (!mOnMapShowMLS) {
return;
}

if ((++count % TIME_CHECK_MULTIPLE == 0) && (SystemClock.uptimeMillis() > endTime)) {
break;
// Draw as a 2nd layer over the observation points
final Point mls = new Point();
revIterator = arrayList.listIterator(mHashedGrid.size());
while (revIterator.hasPrevious()) {
HashMap.Entry<Integer, ObservationPoint> entry = revIterator.previous();
ObservationPoint point = entry.getValue();
if (point.pointMLS != null) {
pj.toPixels(point.pointGPS, gps);
pj.toPixels(point.pointMLS, mls);
drawDot(c, mls, radiusInnerRing - 1, mRedPaint, mBlackStrokePaintThin);
c.drawLine(gps.x, gps.y, mls.x, mls.y, mBlackMLSLinePaint);
}

if ((++count % TIME_CHECK_MULTIPLE == 0) && (SystemClock.uptimeMillis() > endTime)) {
break;
}
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion android/src/main/java/org/mozilla/mozstumbler/client/serialize/KMLFragment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import java.io.FilenameFilter;
import java.lang.ref.WeakReference;
import java.util.LinkedList;
import java.util.List;

public class KMLFragment extends Fragment
implements ObservationPointSerializer.IListener {
Expand All @@ -52,7 +53,10 @@ public class KMLFragment extends Fragment
@Override
public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
mRootView = inflater.inflate(R.layout.fragment_kml, container, false);
mPointsToWrite = ObservedLocationsReceiver.getInstance().getObservationPoints();
List<ObservationPoint> points = ObservedLocationsReceiver.getInstance().getObservationPoints_callerMustLock();
synchronized (points) {
mPointsToWrite = new LinkedList<ObservationPoint>(points);
}
mProgressDialog = new WeakReference<ProgressDialog>(null);
mSavedFileLocation = (TextView) mRootView.findViewById(R.id.textViewSavedFile);

Expand Down

0 comments on commit cb9034c

Please sign in to comment.