From 16322b1422e5dee7804db5d39159cb3c5411e2e4 Mon Sep 17 00:00:00 2001 From: Ray Greenwell Date: Fri, 29 Apr 2005 02:30:23 +0000 Subject: [PATCH] Added some warnings for illegal DSet operations and added a small optimization in the process. Previously, we dumped stack on the server if a entry add was illegal but did nothing for update/remove. On the client, we complained for updates and (indirectly) adds, but not removes. Now we complain everywhere. And- as an added bonus, on the server we now do only one binary search for updates and removals. Whee! git-svn-id: svn+ssh://src.earth.threerings.net/narya/trunk@3528 542714f4-19e9-0310-aa3c-eee0fc999fb1 --- .../com/threerings/presents/dobj/DObject.java | 12 ++++++---- .../com/threerings/presents/dobj/DSet.java | 24 ++++++++++--------- .../presents/dobj/EntryAddedEvent.java | 5 +++- .../presents/dobj/EntryRemovedEvent.java | 16 +++++++++---- .../presents/dobj/EntryUpdatedEvent.java | 8 +++---- 5 files changed, 39 insertions(+), 26 deletions(-) diff --git a/src/java/com/threerings/presents/dobj/DObject.java b/src/java/com/threerings/presents/dobj/DObject.java index 5e8144731..c86bdc455 100644 --- a/src/java/com/threerings/presents/dobj/DObject.java +++ b/src/java/com/threerings/presents/dobj/DObject.java @@ -866,8 +866,10 @@ public class DObject extends TrackedObject // if we're on the authoritative server, we update the set immediately DSet.Entry oldEntry = null; if (_omgr != null && _omgr.isManager(this)) { - oldEntry = set.get(key); - set.removeKey(key); + oldEntry = set.removeKey(key); + if (oldEntry == null) { + Thread.dumpStack(); + } } // dispatch an entry removed event postEvent(new EntryRemovedEvent(_oid, name, key, oldEntry)); @@ -881,8 +883,10 @@ public class DObject extends TrackedObject // if we're on the authoritative server, we update the set immediately DSet.Entry oldEntry = null; if (_omgr != null && _omgr.isManager(this)) { - oldEntry = set.get(entry.getKey()); - set.update(entry); + oldEntry = set.update(entry); + if (oldEntry == null) { + Thread.dumpStack(); + } } // dispatch an entry updated event postEvent(new EntryUpdatedEvent(_oid, name, entry, oldEntry)); diff --git a/src/java/com/threerings/presents/dobj/DSet.java b/src/java/com/threerings/presents/dobj/DSet.java index 17d546440..a15683c40 100644 --- a/src/java/com/threerings/presents/dobj/DSet.java +++ b/src/java/com/threerings/presents/dobj/DSet.java @@ -295,7 +295,7 @@ public class DSet */ protected boolean remove (Entry elem) { - return removeKey(elem.getKey()); + return (null != removeKey(elem.getKey())); } /** @@ -304,10 +304,10 @@ public class DSet * removeFrom{Set}() method should be called on the * distributed object that contains the set in question. * - * @return true if a matching entry was removed, false if no entry - * in the set matched the key. + * @return the old matching entry if found and removed, null if not + * found. */ - protected boolean removeKey (Object key) + protected Entry removeKey (Object key) { // look up this entry's position in our set int eidx = ArrayUtil.binarySearch( @@ -316,13 +316,14 @@ public class DSet // if we found it, remove it if (eidx >= 0) { // shift the remaining elements down + Entry oldEntry = _entries[eidx]; System.arraycopy(_entries, eidx+1, _entries, eidx, _size-eidx-1); _entries[--_size] = null; _modCount++; - return true; + return oldEntry; } else { - return false; + return null; } } @@ -333,10 +334,10 @@ public class DSet * update{Set}() method should be called on the * distributed object that contains the set in question. * - * @return true if the entry was updated, false if it was not - * already in the set (in which case nothing is updated). + * @return the old entry that was replaced, or null if it was not + * found (in which case nothing is updated). */ - protected boolean update (Entry elem) + protected Entry update (Entry elem) { // look up this entry's position in our set int eidx = ArrayUtil.binarySearch( @@ -344,11 +345,12 @@ public class DSet // if we found it, update it if (eidx >= 0) { + Entry oldEntry = _entries[eidx]; _entries[eidx] = elem; _modCount++; - return true; + return oldEntry; } else { - return false; + return null; } } diff --git a/src/java/com/threerings/presents/dobj/EntryAddedEvent.java b/src/java/com/threerings/presents/dobj/EntryAddedEvent.java index 6190cff49..26eed49d7 100644 --- a/src/java/com/threerings/presents/dobj/EntryAddedEvent.java +++ b/src/java/com/threerings/presents/dobj/EntryAddedEvent.java @@ -84,7 +84,10 @@ public class EntryAddedEvent extends NamedEvent throws ObjectAccessException { if (!_alreadyApplied) { - ((DSet)target.getAttribute(_name)).add(_entry); + boolean added = ((DSet)target.getAttribute(_name)).add(_entry); + if (!added) { + Log.warning("Duplicate entry found [event=" + this + "]."); + } } return true; } diff --git a/src/java/com/threerings/presents/dobj/EntryRemovedEvent.java b/src/java/com/threerings/presents/dobj/EntryRemovedEvent.java index c735622f6..bb2466078 100644 --- a/src/java/com/threerings/presents/dobj/EntryRemovedEvent.java +++ b/src/java/com/threerings/presents/dobj/EntryRemovedEvent.java @@ -1,5 +1,5 @@ // -// $Id: EntryRemovedEvent.java,v 1.17 2004/08/27 02:20:20 mdb Exp $ +// $Id$ // // Narya library - tools for developing networked games // Copyright (C) 2002-2004 Three Rings Design, Inc., All Rights Reserved @@ -21,6 +21,8 @@ package com.threerings.presents.dobj; +import com.threerings.presents.Log; + /** * An entry removed event is dispatched when an entry is removed from a * {@link DSet} attribute of a distributed object. It can also be @@ -82,10 +84,14 @@ public class EntryRemovedEvent extends NamedEvent { if (_oldEntry == UNSET_OLD_ENTRY) { DSet set = (DSet)target.getAttribute(_name); - // fetch the previous value for interested callers - _oldEntry = set.get(_key); - // remove it from the set - set.removeKey(_key); + // remove, fetch the previous value for interested callers + _oldEntry = set.removeKey(_key); + if (_oldEntry == null) { + // complain if there was actually nothing there + Log.warning("No matching entry to remove [key=" + _key + + ", set=" + set + "]."); + return false; + } } return true; } diff --git a/src/java/com/threerings/presents/dobj/EntryUpdatedEvent.java b/src/java/com/threerings/presents/dobj/EntryUpdatedEvent.java index 401a76527..35f5561e7 100644 --- a/src/java/com/threerings/presents/dobj/EntryUpdatedEvent.java +++ b/src/java/com/threerings/presents/dobj/EntryUpdatedEvent.java @@ -1,5 +1,5 @@ // -// $Id: EntryUpdatedEvent.java,v 1.13 2004/08/27 02:20:20 mdb Exp $ +// $Id$ // // Narya library - tools for developing networked games // Copyright (C) 2002-2004 Three Rings Design, Inc., All Rights Reserved @@ -87,10 +87,8 @@ public class EntryUpdatedEvent extends NamedEvent if (_oldEntry == UNSET_OLD_ENTRY) { DSet set = (DSet)target.getAttribute(_name); // fetch the previous value for interested callers - _oldEntry = set.get(_entry.getKey()); - - // update the entry - if (!set.update(_entry)) { + _oldEntry = set.update(_entry); + if (_oldEntry == null) { // complain if we didn't update anything Log.warning("No matching entry to update [entry=" + this + ", set=" + set + "].");