horrific cursor code gone

3 files changed, 66 insertions, 99 deletions

diff --git a/db/clientcursor.cpp b/db/clientcursor.cpp
index ce431cdeed3..d113bba9f78 100644
--- a/db/clientcursor.cpp
+++ b/db/clientcursor.cpp
@@ -1,9 +1,9 @@
-// clientcursor.cpp
-
-/* Cursor -- and its derived classes -- are our internal cursors.
+/* clientcursor.cpp
 
    ClientCursor is a wrapper that represents a cursorid from our client 
    application's perspective.
+
+   Cursor -- and its derived classes -- are our internal cursors.
 */
 
 #include "stdafx.h"
@@ -13,11 +13,30 @@
 
 /* TODO: FIX cleanup of clientCursors when hit the end. (ntoreturn insufficient) */
 
-typedef map<DiskLoc, set<ClientCursor*>> DiskLocToCC;
-DiskLocToCC clientCursorsByLocation;
-
 CCById clientCursorsById;
 
+/* ------------------------------------------- */
+
+typedef multimap<DiskLoc, ClientCursor*> ByLoc;
+ByLoc byLoc;
+
+void ClientCursor::setLastLoc(DiskLoc L) { 
+	if( L == _lastLoc ) 
+		return;
+
+	if( !_lastLoc.isNull() ) {
+		ByLoc::iterator i = kv_find(byLoc, _lastLoc, this);
+		if( i != byLoc.end() )
+			byLoc.erase(i);
+	}
+
+	if( !L.isNull() )
+		byLoc.insert( make_pair(L, this) );
+	_lastLoc = L;
+}
+
+/* ------------------------------------------- */
+
 /* must call this when a btree node is updated */
 void removedKey(const DiskLoc& btreeLoc, int keyPos) { 
 // TODO!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
@@ -25,56 +44,31 @@ void removedKey(const DiskLoc& btreeLoc, int keyPos) {
 
 /* must call this on a delete so we clean up the cursors. */
 void aboutToDelete(const DiskLoc& dl) { 
-	DiskLocToCC::iterator it = clientCursorsByLocation.find(dl);
-
-	if( it != clientCursorsByLocation.end() ) {
-		set<ClientCursor*>& ccs = it->second;
-
-		set<ClientCursor*>::iterator it = ccs.begin();
-		while( it != ccs.end() ) { 
-			ClientCursor *cc = *it;
-			cc->c->checkLocation();
-			cc->c->advance();
-			assert( cc->currLoc() != dl ); // assert that we actually advanced
-			cc->lastLoc.Null(); //  updateLocation must not try to remove, we are cleaining up this list ourself.
-			cc->updateLocation();
-		}
+	vector<ClientCursor*> toAdvance;
 
-		clientCursorsByLocation.erase(it);
+	for( ByLoc::iterator i = byLoc.lower_bound(dl); 
+		i != byLoc.upper_bound(dl); ++i ) { 
+			toAdvance.push_back(i->second);
 	}
-}
-
-void ClientCursor::cleanupByLocation(DiskLoc loc) { 
-	if( loc.isNull() )
-		return;
 
-	DiskToCC::iterator it = byLocation.find(loc);
-	if( it != byLocation.end() ) {
-		it->second.erase(this);
-		if( it->second.empty() )
-			it->erase();
+	for( vector<ClientCursor*>::iterator i = toAdvance.begin();
+		i != toAdvance.end(); ++i ) 
+	{ 
+		(*i)->c->checkLocation();
+		(*i)->c->advance();
+		wassert( (*i)->c->currLoc() != dl );
+		(*i)->updateLocation();
 	}
 }
 
 ClientCursor::~ClientCursor() {
-#if defined(_WIN32)
-	cout << "~clientcursor " << cursorid << endl;
-#endif
-	assert( pos != -2 );
-	cleanupByLocation(lastLoc);
+	DEV cout << "~clientcursor " << cursorid << endl;
 	assert( pos != -2 );
-
-	// defensive
-	lastLoc.Null();
-	cursorid = -1; 
+	setLastLoc( DiskLoc() ); // removes us from bylocation multimap
+	clientCursorsById.erase(cursorid);
+	// defensive:
+	(CursorId&) cursorid = -1; 
 	pos = -2;
-	nextAtThisLocation = 0;
-}
-
-// note this doesn't set lastLoc -- caller should.
-void ClientCursor::addToByLocation(DiskLoc cl) { 
-	assert( cursorid );
-	clientCursorsByLocation[cl].insert(this);
 }
 
 /* call when cursor's location changes so that we can update the 
@@ -84,53 +78,23 @@ void ClientCursor::addToByLocation(DiskLoc cl) {
 void ClientCursor::updateLocation() {
 	assert( cursorid );
 	DiskLoc cl = c->currLoc();
-
-	if( lastLoc == cl ) {
+	if( lastLoc() == cl ) {
 		cout << "info: lastloc==curloc " << ns << '\n';
 		return;
 	}
-
-	if( !lastLoc.isNull() )
-		cleanupByLocation(lastLoc, cursorid);
-
-	if( !cl.isNull() )
-		addToByLocation(cl);
-
-	lastLoc = cl;
+	setLastLoc(cl);
 	c->noteLocation();
 }
 
-/* report to us that a new clientcursor exists so we can track it. 
-   note you still must call updateLocation (which likely should be changed)
-*/
-void ClientCursor::add(ClientCursor* cc) {
-	clientCursors[cc->cursorid] = cc;
-	updateLocation();
-}
-
-bool ClientCursor::erase(long long id) { 
-	CCById::iterator it = clientCursorsById.find(id);
-	if( it != clientCursorsById.end() ) {
-		ClientCursor *cc = it->second;
-		it->second = 0; // defensive
-		clientCursorsById.erase(it);
-		delete cc; // destructor will fix byLocation map
-		return true;
-	}
-	return false;
-}
-
-long long allocCursorId() { 
+long long ClientCursor::allocCursorId() { 
 	long long x;
 	while( 1 ) {
 		x = (((long long)rand()) << 32);
 		x = x | (int) curTimeMillis() | 0x80000000; // OR to make sure not zero
-		if( clientCursors.count(x) == 0 )
+		if( ClientCursor::find(x) == 0 )
 			break;
 	}
-#if defined(_WIN32)
-	cout << "alloccursorid " << x << endl;
-#endif
+	DEV cout << "alloccursorid " << x << endl;
 	return x;
 }
 
@@ -139,9 +103,9 @@ class CursInspector : public SingleResultObjCursor {
 		return new CursInspector(); 
 	}
 	void fill() { 
-		b.append("byLocation_size", clientCursorsByLocation.size());
+		b.append("byLocation_size", byLoc.size());
 		b.append("clientCursors_size", clientCursorsById.size());
-
+/* todo update for new impl:
 		stringstream ss;
 		ss << '\n';
 		int x = 40;
@@ -172,6 +136,7 @@ class CursInspector : public SingleResultObjCursor {
 			it++;
 		}
 		b.append("dump", ss.str().c_str());
+*/
 	}
 public:
 	CursInspector() { reg("intr.cursors"); }
diff --git a/db/clientcursor.h b/db/clientcursor.h
index a0d06f8557c..6fd5bb12155 100644
--- a/db/clientcursor.h
+++ b/db/clientcursor.h
@@ -16,28 +16,33 @@ class ClientCursor;
 typedef map<CursorId, ClientCursor*> CCById;
 extern CCById clientCursorsById;
 
+
 class ClientCursor {
 	friend class CursInspector;
+	DiskLoc _lastLoc; // use getter and setter not this.
+	static CursorId allocCursorId();
 public:
-	ClientCursor() { 
-		cursorid=0; pos=0; 
+	ClientCursor() : cursorid( allocCursorId() ), pos(0) { 
+		clientCursorsById.insert( make_pair(cursorid, this) );
 	}
 	~ClientCursor();
-	CursorId cursorid;
+	const CursorId cursorid;
 	string ns;
 	auto_ptr<JSMatcher> matcher;
 	auto_ptr<Cursor> c;
 	int pos;
-	DiskLoc lastLoc;
+	DiskLoc lastLoc() const { return _lastLoc; }
+	void setLastLoc(DiskLoc);
 	auto_ptr< set<string> > filter; // which fields query wants returned
 
-	/* report to us that a new clientcursor exists so we can track it.
-	   note you do not need to call updateLocation, but location should be set before
-	   calling.
-	   */
-	static void add(ClientCursor*);
-
-	static bool erase(CursorId cursorid);
+	static bool erase(CursorId id) { 
+		ClientCursor *cc = find(id);
+		if( cc ) {
+			delete cc; 
+			return true;
+		}
+		return false;
+	}
 
 	static ClientCursor* find(CursorId id) {
 		CCById::iterator it = clientCursorsById.find(id);
@@ -61,4 +66,3 @@ public:
 //	ClientCursor *nextAtThisLocation;
 };
 
-CursorId allocCursorId();
diff --git a/db/query.cpp b/db/query.cpp
index 913a6612a5d..bfef39534fd 100644
--- a/db/query.cpp
+++ b/db/query.cpp
@@ -732,13 +732,11 @@ assert( debug.getN() < 5000 );
 											// more...so save a cursor
 											ClientCursor *cc = new ClientCursor();
 											cc->c = c;
-											cursorid = allocCursorId();
-											cc->cursorid = cursorid;
+											cursorid = cc->cursorid;
 											cc->matcher = matcher;
 											cc->ns = ns;
 											cc->pos = n;
 											cc->filter = filter;
-											ClientCursor::add(cc);
 											cc->updateLocation();
 										}
 									}