adts: optimize landlord algorithm at advisor's request (only relevant if we have tons of files in cache)

trace: make note of IO size

xmlutils: fix old remnant of Handle vfs_load return type.

This was SVN commit r3456.

source/lib/adts.h

@@ -222,12 +222,19 @@ public:
 template<typename Key, typename T> class Cache
 {
 public:
+	Cache()
+		: min_credit_density(FLT_MAX) {}
+
 	void add(Key key, T item, size_t size, uint cost)
 	{
 		typedef std::pair<CacheMapIt, bool> PairIB;
 		typename CacheMap::value_type val = std::make_pair(key, CacheEntry(item, size, cost));
 		PairIB ret = map.insert(val);
 		debug_assert(ret.second);	// must not already be in map
+
+		// adding new item - min_credit_density may decrease
+		const CacheEntry& new_entry = ret.first->second;
+		notify_credit_reduced(new_entry);
 	}
 
 	// remove the entry identified by <key>. expected usage is to check
@@ -236,7 +243,21 @@ public:
 	// useful for invalidating single cache entries.
 	void remove(Key key)
 	{
-		map.erase(key);
+		CacheMapIt it = map.find(key);
+		if(it == map.end())
+		{
+			debug_warn("Cache: item to be removed not found");
+			return;
+		}
+
+		// we're removing. if this one had the smallest
+		// density, recalculate.
+		const bool need_recalc = is_min_entry(it->second);
+
+		map.erase(it);
+
+		if(need_recalc)
+			recalc_min_density();
 	}
 
 	// if there is no entry for <key> in the cache, return 0 with
@@ -253,10 +274,17 @@ public:
 
 		if(refill_credit)
 		{
+			// we're increasing credit. if this one had the smallest
+			// density, recalculate.
+			const bool need_recalc = is_min_entry(entry);
+
 			// Landlord algorithm calls for credit to be reset to anything
 			// between its current value and the cost.
 			const float gain = 0.75f;	// restore most credit
 			entry.credit = gain*entry.cost + (1.0f-gain)*entry.credit;
+
+			if(need_recalc)
+				recalc_min_density();
 		}
 
 		return entry.item;
@@ -267,28 +295,22 @@ public:
 	// how big it was (useful for statistics).
 	T remove_least_valuable(size_t* psize = 0)
 	{
-		CacheMapIt it;
-
 		// one iteration ought to suffice to evict someone due to
 		// definition of min_density, but we provide for repeating
 		// in case of floating-point imprecision.
 		// (goto vs. loop avoids nesting and emphasizes rarity)
 again:	
 
-		// find minimum credit density (needed for charge step)
-		float min_density = 1e10;	// = \delta in [Young02]
-		for( it = map.begin(); it != map.end(); ++it)
+		// charge everyone rent (proportional to min_credit_density and size)
+		// .. latch current delta value to avoid it changing during the loop
+		//    (due to notify_* calls). this ensures fairness.
+		const float delta = min_credit_density;
+		for(CacheMapIt it = map.begin(); it != map.end(); ++it)
 		{
 			CacheEntry& entry = it->second;
-			const float density = entry.credit / entry.size;
-			min_density = MIN(density, min_density);
-		}
-
-		// .. charge everyone rent (proportional to min_density and size)
-		for( it = map.begin(); it != map.end(); ++it)
-		{
-			CacheEntry& entry = it->second;
-			entry.credit -= min_density * entry.size;
+			entry.credit -= delta * entry.size;
+			// reducing credit - min_credit_density may decrease
+			notify_credit_reduced(entry);
 
 			// evict immediately if credit is exhausted
 			// (note: Landlord algorithm calls for 'any subset' of
@@ -304,6 +326,9 @@ again:
 				if(psize)
 					*psize = entry.size;
 				map.erase(it);
+				// this item had the least density, else it wouldn't
+				// have been removed. recalculate.
+				recalc_min_density();
 				return item;
 			}
 		}
@@ -313,28 +338,52 @@ again:
 	}
 
 private:
-	class CacheEntry
+	struct CacheEntry
 	{
-		friend class Cache;
+		T item;
+		size_t size;
+		float size_reciprocal;
+		uint cost;
+		float credit;
 
 		CacheEntry(T item_, size_t size_, uint cost_)
+			: item(item_)
 		{
-			item = item_;
-
 			size = size_;
+			size_reciprocal = 1.0f / size;
+
 			cost = cost_;
 			credit = cost;
 		}
-
-		T item;
-		size_t size;
-		uint cost;
-		float credit;
 	};
 
-	typedef std::map<Key, CacheEntry> CacheMap;
+	// note: use hash_map instead of map for better locality
+	// (relevant when iterating over all items in remove_least_valuable)
+	typedef STL_HASH_MAP<Key, CacheEntry> CacheMap;
 	typedef typename CacheMap::iterator CacheMapIt;
 	CacheMap map;
+
+	// = \delta in [Young02] (needed for charge step)
+	// this is cached to avoid having to iterate over the whole map.
+	float min_credit_density;
+	float credit_density(const CacheEntry& entry)
+	{
+		return entry.credit * entry.size_reciprocal;
+	}
+	void notify_credit_reduced(const CacheEntry& entry)
+	{
+		min_credit_density = MIN(min_credit_density, credit_density(entry));
+	}
+	bool is_min_entry(const CacheEntry& entry)
+	{
+		return feq(min_credit_density, credit_density(entry));
+	}
+	void recalc_min_density()
+	{
+		min_credit_density = FLT_MAX;
+		for(CacheMapIt it = map.begin(); it != map.end(); ++it)
+			min_credit_density = MIN(min_credit_density, credit_density(it->second));
+	}
 };
 
 

source/lib/allocators.cpp

@@ -168,10 +168,9 @@ static LibError mem_release(u8* p, size_t size)
 static LibError mem_commit(u8* p, size_t size, int prot)
 {
 	if(prot == PROT_NONE)
-	{
-		debug_warn("mem_commit: prot=PROT_NONE isn't allowed (misinterpreted by mmap)");
-		return ERR_INVALID_PARAM;
-	}
+		// not allowed - it would be misinterpreted by mmap.
+		WARN_RETURN(ERR_INVALID_PARAM);
+
 	errno = 0;
 	void* ret = mmap(p, size, prot, mmap_flags|MAP_FIXED, -1, 0);
 	return LibError_from_mmap(ret);

source/lib/res/file/file_cache.cpp

@@ -653,7 +653,7 @@ LibError file_buf_free(FileIOBuf buf)
 	extant_bufs.find_and_remove(buf, &size, &atom_fn);
 
 	stats_buf_free();
-	trace_notify_free(atom_fn);
+	trace_notify_free(atom_fn, size);
 
 	return ERR_OK;
 }

source/lib/res/file/vfs.cpp

@@ -424,7 +424,7 @@ ssize_t vfs_io(const Handle hf, const size_t size, FileIOBuf* pbuf,
 	FileCommon* fc = &vf->xf.u.fc;
 
 	stats_user_io(size);
-	trace_notify_load(fc->atom_fn, fc->flags);
+	trace_notify_load(fc->atom_fn, size, fc->flags);
 
 	off_t ofs = vf->ofs;
 	vf->ofs += (off_t)size;
@@ -449,7 +449,7 @@ LibError vfs_load(const char* V_fn, FileIOBuf& buf, size_t& size, uint flags /*
 		// efficiency. that includes stats/trace accounting, though,
 		// so duplicate that here:
 		stats_user_io(size);
-		trace_notify_load(atom_fn, flags);
+		trace_notify_load(atom_fn, size, flags);
 		return ERR_OK;
 	}
 

source/lib/res/file/vfs_optimizer.cpp

@@ -29,7 +29,7 @@ void trace_enable(bool want_enabled)
 }
 
 
-static void trace_add(TraceOp op, const char* P_fn, uint flags = 0, double timestamp = 0.0)
+static void trace_add(TraceOp op, const char* P_fn, size_t size, uint flags = 0, double timestamp = 0.0)
 {
 	trace_init();
 	if(!trace_enabled)
@@ -42,20 +42,21 @@ static void trace_add(TraceOp op, const char* P_fn, uint flags = 0, double times
 	if(!t)
 		return;
 	t->timestamp = timestamp;
-	t->atom_fn = file_make_unique_fn_copy(P_fn);
-	t->op = op;
-	t->flags = flags;
+	t->atom_fn   = file_make_unique_fn_copy(P_fn);
+	t->size      = size;
+	t->op        = op;
+	t->flags     = flags;
 }
 
 
-void trace_notify_load(const char* P_fn, uint flags)
+void trace_notify_load(const char* P_fn, size_t size, uint flags)
 {
-	trace_add(TO_LOAD, P_fn, flags);
+	trace_add(TO_LOAD, P_fn, size, flags);
 }
 
-void trace_notify_free(const char* P_fn)
+void trace_notify_free(const char* P_fn, size_t size)
 {
-	trace_add(TO_FREE, P_fn);
+	trace_add(TO_FREE, P_fn, size);
 }
 
 
@@ -93,7 +94,7 @@ LibError trace_write_to_file(const char* trace_filename)
 		}
 
 		debug_assert(ent->op == TO_LOAD || ent->op == TO_FREE);
-		fprintf(f, "%#010f: %c \"%s\" %02x\n", ent->timestamp, opcode, ent->atom_fn, ent->flags);
+		fprintf(f, "%#010f: %c \"%s\" %d %04x\n", ent->timestamp, opcode, ent->atom_fn, ent->size, ent->flags);
 	}
 
 	(void)fclose(f);
@@ -118,15 +119,14 @@ LibError trace_read_from_file(const char* trace_filename, Trace* t)
 	trace_clear();
 	// .. bake PATH_MAX limit into string.
 	char fmt[30];
-	snprintf(fmt, ARRAY_SIZE(fmt), "%%lf: %%c \"%%%d[^\"]\" %%02x\n", PATH_MAX);
+	snprintf(fmt, ARRAY_SIZE(fmt), "%%lf: %%c \"%%%d[^\"]\" %%d %%04x\n", PATH_MAX);
 	for(;;)
 	{
-		double timestamp; char opcode; char P_path[PATH_MAX];
-		uint flags = 0;	// optional
-		int ret = fscanf(f, fmt, &timestamp, &opcode, P_path, &flags);
+		double timestamp; char opcode; char P_path[PATH_MAX]; size_t size; uint flags;
+		int ret = fscanf(f, fmt, &timestamp, &opcode, P_path, &size, &flags);
 		if(ret == EOF)
 			break;
-		debug_assert(ret == 4);
+		debug_assert(ret == 5);
 
 		TraceOp op = TO_LOAD;	// default in case file is garbled
 		switch(opcode)
@@ -136,7 +136,7 @@ LibError trace_read_from_file(const char* trace_filename, Trace* t)
 		default: debug_warn("invalid TraceOp");
 		}
 
-		trace_add(op, P_path, flags, timestamp);
+		trace_add(op, P_path, size, flags, timestamp);
 	}
 
 	fclose(f);

source/lib/res/file/vfs_optimizer.h

@@ -4,8 +4,8 @@
 extern void trace_enable(bool want_enabled);
 extern void trace_shutdown();
 
-extern void trace_notify_load(const char* P_fn, uint flags);
-extern void trace_notify_free(const char* P_fn);
+extern void trace_notify_load(const char* P_fn, size_t size, uint flags);
+extern void trace_notify_free(const char* P_fn, size_t size);
 
 // TraceEntry operation type.
 // note: rather than only a list of accessed files, we also need to
@@ -24,8 +24,13 @@ enum TraceOp
 // (to prevent trace file writes from affecting other IOs)
 struct TraceEntry
 {
-	double timestamp;		// returned by get_time before operation starts
+	// note: float instead of double for nice 16 byte struct size
+	float timestamp;		// returned by get_time before operation starts
 	const char* atom_fn;	// path+name of affected file
+	// rationale: store size in the trace because other applications
+	// that use this trace format but not our IO code wouldn't know
+	// size (since they cannot retrieve the file info given atom_fn).
+	size_t size;			// of IO (usually the entire file)
 	uint op : 8;			// operation - see TraceOp
 	uint flags : 24;		// misc, e.g. file_io flags.
 };

source/ps/XML/XMLUtils.cpp

@@ -84,7 +84,7 @@ CVFSInputSource::~CVFSInputSource()
 
 BinInputStream *CVFSInputSource::makeStream() const
 {
-	if (m_pBuffer > 0)
+	if (m_pBuffer != 0)
 	{
 #include "nommgr.h"
 		return new BinMemInputStream((XMLByte *)m_pBuffer, (unsigned int)m_BufferSize,