Changeset 9267 in josm for trunk/src/org
- Timestamp:
- 2016-01-02T19:15:51+01:00 (9 years ago)
- Location:
- trunk/src/org/openstreetmap/josm/data/osm
- Files:
-
- 2 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java
r9243 r9267 482 482 ------------*/ 483 483 484 // Note that all methods that read keys first make local copy of keys array reference. This is to ensure thread safety - reading485 // doesn't have to be locked so it's possible that keys array will be modified. But all write methods make copy of keys array so486 // the array itself will be never modified - only reference will be changed487 488 484 /** 489 485 * The key/value list for this primitive. 490 */ 491 protected String[] keys; 486 * <p> 487 * Note that the keys field is synchronized using RCU. 488 * Writes to it are not synchronized by this object, the writers have to synchronize writes themselves. 489 * <p> 490 * In short this means that you should not rely on this variable being the same value when read again and your should always 491 * copy it on writes. 492 * <p> 493 * Further reading: 494 * <ul> 495 * <li>{@link java.util.concurrent.CopyOnWriteArrayList}</li> 496 * <li> <a href="http://stackoverflow.com/questions/2950871/how-can-copyonwritearraylist-be-thread-safe"> 497 * http://stackoverflow.com/questions/2950871/how-can-copyonwritearraylist-be-thread-safe</a></li> 498 * <li> <a href="https://en.wikipedia.org/wiki/Read-copy-update"> 499 * https://en.wikipedia.org/wiki/Read-copy-update</a> (mind that we have a Garbage collector, 500 * {@code rcu_assign_pointer} and {@code rcu_dereference} are ensured by the {@code volatile} keyword)</li> 501 * </ul> 502 */ 503 protected volatile String[] keys; 492 504 493 505 /** … … 531 543 * Old key/value pairs are removed. 532 544 * If <code>keys</code> is null, clears existing key/value pairs. 545 * <p> 546 * Note that this method, like all methods that modify keys, is not synchronized and may lead to data corruption when being used 547 * from multiple threads. 533 548 * 534 549 * @param keys the key/value pairs to set. If null, removes all existing key/value pairs. … … 555 570 * Set the given value to the given key. If key is null, does nothing. If value is null, 556 571 * removes the key and behaves like {@link #remove(String)}. 572 * <p> 573 * Note that this method, like all methods that modify keys, is not synchronized and may lead to data corruption when being used 574 * from multiple threads. 557 575 * 558 576 * @param key The key, for which the value is to be set. Can be null or empty, does nothing in this case. … … 572 590 keysChangedImpl(originalKeys); 573 591 } else { 574 for (int i = 0; i < keys.length; i += 2) { 575 if (keys[i].equals(key)) { 576 // This modifies the keys array but it doesn't make it invalidate for any time so its ok (see note no top) 577 keys[i+1] = value; 578 keysChangedImpl(originalKeys); 579 return; 580 } 592 int keyIndex = indexOfKey(keys, key); 593 int tagArrayLength = keys.length; 594 if (keyIndex < 0) { 595 keyIndex = tagArrayLength; 596 tagArrayLength += 2; 581 597 } 582 String[] newKeys = Arrays.copyOf(keys, keys.length + 2); 583 newKeys[keys.length] = key; 584 newKeys[keys.length + 1] = value; 598 599 // Do not try to optimize this array creation if the key already exists. 600 // We would need to convert the keys array to be an AtomicReferenceArray 601 // Or we would at least need a volatile write after the array was modified to 602 // ensure that changes are visible by other threads. 603 String[] newKeys = Arrays.copyOf(keys, tagArrayLength); 604 newKeys[keyIndex] = key; 605 newKeys[keyIndex + 1] = value; 585 606 keys = newKeys; 586 607 keysChangedImpl(originalKeys); … … 589 610 590 611 /** 612 * Scans a key/value array for a given key. 613 * @param keys The key array. It is not modified. It may be null to indicate an emtpy array. 614 * @param key The key to search for. 615 * @return The position of that key in the keys array - which is always a multiple of 2 - or -1 if it was not found. 616 */ 617 private static int indexOfKey(String[] keys, String key) { 618 if (keys == null) { 619 return -1; 620 } 621 for (int i = 0; i < keys.length; i += 2) { 622 if (keys[i].equals(key)) { 623 return i; 624 } 625 } 626 return -1; 627 } 628 629 /** 591 630 * Remove the given key from the list 631 * <p> 632 * Note that this method, like all methods that modify keys, is not synchronized and may lead to data corruption when being used 633 * from multiple threads. 592 634 * 593 635 * @param key the key to be removed. Ignored, if key is null. … … 618 660 /** 619 661 * Removes all keys from this primitive. 662 * <p> 663 * Note that this method, like all methods that modify keys, is not synchronized and may lead to data corruption when being used 664 * from multiple threads. 620 665 */ 621 666 @Override … … 681 726 682 727 public final int getNumKeys() { 728 String[] keys = this.keys; 683 729 return keys == null ? 0 : keys.length / 2; 684 730 } … … 719 765 */ 720 766 public boolean hasKey(String key) { 721 String[] keys = this.keys; 722 if (key == null) return false; 723 if (keys == null) return false; 724 for (int i = 0; i < keys.length; i += 2) { 725 if (keys[i].equals(key)) return true; 726 } 727 return false; 767 return key != null && indexOfKey(keys, key) >= 0; 728 768 } 729 769 -
trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java
r9243 r9267 776 776 /** 777 777 * Returns {@link #getKeys()} for which {@code key} does not fulfill {@link #isUninterestingKey}. 778 * @return listof interesting tags778 * @return A map of interesting tags 779 779 */ 780 780 public Map<String, String> getInterestingTags() { … … 833 833 834 834 private void updateTagged() { 835 if (keys != null) { 836 for (String key: keySet()) { 837 // 'area' is not really uninteresting (putting it in that list may have unpredictable side effects) 838 // but it's clearly not enough to consider an object as tagged (see #9261) 839 if (!isUninterestingKey(key) && !"area".equals(key)) { 840 updateFlagsNoLock(FLAG_TAGGED, true); 841 return; 842 } 835 for (String key: keySet()) { 836 // 'area' is not really uninteresting (putting it in that list may have unpredictable side effects) 837 // but it's clearly not enough to consider an object as tagged (see #9261) 838 if (!isUninterestingKey(key) && !"area".equals(key)) { 839 updateFlagsNoLock(FLAG_TAGGED, true); 840 return; 843 841 } 844 842 } … … 847 845 848 846 private void updateAnnotated() { 849 if (keys != null) { 850 for (String key: keySet()) { 851 if (getWorkInProgressKeys().contains(key)) { 852 updateFlagsNoLock(FLAG_ANNOTATED, true); 853 return; 854 } 847 for (String key: keySet()) { 848 if (getWorkInProgressKeys().contains(key)) { 849 updateFlagsNoLock(FLAG_ANNOTATED, true); 850 return; 855 851 } 856 852 } … … 1188 1184 */ 1189 1185 public boolean hasSameInterestingTags(OsmPrimitive other) { 1190 // We cannot directly use Arrays.equals(keys, other.keys) as keys is not ordered by key1191 // but we can at least check if both arrays are null or of the same size before creating1192 // and comparing the key maps (costly operation, see #7159)1193 1186 return (keys == null && other.keys == null) 1194 || (keys != null && other.keys != null && keys.length == other.keys.length 1195 && (keys.length == 0 || getInterestingTags().equals(other.getInterestingTags()))); 1187 || getInterestingTags().equals(other.getInterestingTags()); 1196 1188 } 1197 1189
Note:
See TracChangeset
for help on using the changeset viewer.