Bees: fix [-Werror=implicit-fallthrough=]

In gcc 7+ warning: implicit-fallthrough has been added
In some places fallthrough is expectable, disable warning

Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com>

Makefile

@@ -1,19 +1,36 @@
-default install all: lib src test README.html
+PREFIX ?= /
 
-clean:
+default all: lib src test README.html
+
+clean: ## Cleanup
 	git clean -dfx
 
 .PHONY: lib src
 
-lib:
+lib: ## Build libs
 	$(MAKE) -C lib
 
+src: ## Build bins
 src: lib
 	$(MAKE) -C src
 
+test: ## Run tests
 test: lib src
 	$(MAKE) -C test
 
 README.html: README.md
 	markdown README.md > README.html.new
 	mv -f README.html.new README.html
+
+install: ## Install bees + libs
+install: lib src test
+	install -Dm644 lib/libcrucible.so $(PREFIX)/usr/lib/libcrucible.so
+	install -Dm755 bin/bees $(PREFIX)/usr/bin/bees
+
+install_scripts: ## Install scipts
+	install -Dm755 scripts/beesd $(PREFIX)/usr/bin/beesd
+	install -Dm644 scripts/beesd.conf.sample $(PREFIX)/etc/bees/beesd.conf.sample
+	install -Dm644 scripts/beesd@.service $(PREFIX)/lib/systemd/system/beesd@.service
+
+help: ## Show help
+	@fgrep -h "##" $(MAKEFILE_LIST) | fgrep -v fgrep | sed -e 's/\\$$//' | sed -e 's/##/\t/'

README.md

@@ -234,22 +234,19 @@ Bug fixes (sometimes included in older LTS kernels):
 * 4.5: use-after-free in the `FILE_EXTENT_SAME` ioctl used by Bees.
 * 4.7: *slow backref* bug no longer triggers a softlockup panic.  It still
   too long to resolve a block address to a root/inode/offset triple.
-
+* 4.10-rc1: reduced CPU time cost of the LOGICAL_INO ioctl and dedup
-Fixed bugs not yet integrated in mainline Linux:
+  backref processing in general.
-
-* 7f8e406 ("btrfs: improve delayed refs iterations"): significantly
-  reduces the CPU time cost of the LOGICAL_INO ioctl (from 30-70% of
-  bees running time to under 5%).
 
 Unfixed kernel bugs (as of 4.5.7) with workarounds in Bees:
 
-* *slow backref*: If the number of references to a single shared extent
+* *slow backrefs* (aka toxic extents): If the number of references to a
-  within a single file grows above a few thousand, the kernel consumes CPU
+  single shared extent within a single file grows above a few thousand,
-  for up to 40 uninterruptible minutes while holding various locks that
+  the kernel consumes CPU for minutes at a time while holding various
-  block access to the filesystem.  Bees avoids this bug by measuring the
+  locks that block access to the filesystem.  Bees avoids this bug by
-  time the kernel spends performing certain operations and permanently
+  measuring the time the kernel spends performing certain operations
-  blacklisting any extent or hash where the kernel starts to get slow.
+  and permanently blacklisting any extent or hash where the kernel
-  Inside Bees, such blocks are marked as 'toxic' hash/block addresses.
+  starts to get slow.  Inside Bees, such blocks are marked as 'toxic'
+  hash/block addresses.
 
 * `LOGICAL_INO` output is arbitrarily limited to 2730 references
   even if more buffer space is provided for results.  Once this number
@@ -316,22 +313,14 @@ Requirements
 
 * libuuid-dev
 
-  TODO: remove the one function used from this library.
+  This library is only required for a feature that was removed after v0.1.
-  It supports a feature Bees no longer implements.
+  The lingering support code can be removed.
 
 * Linux kernel 4.4.3 or later
 
   Don't bother trying to make Bees work with older kernels.
   It won't end well.
 
-* 64-bit host and target CPU
-
-  This code has never been tested on a 32-bit target CPU.
-
-  A 64-bit host CPU may be required for the self-tests.
-  Some of the ioctls don't work properly with a 64-bit
-  kernel and 32-bit userspace.
-
 Build
 -----
 
@@ -441,6 +430,6 @@ You can also use Github:
 Copyright & License
 ===================
 
-Copyright 2015-2016 Zygo Blaxell <bees@furryterror.org>.
+Copyright 2015-2017 Zygo Blaxell <bees@furryterror.org>.
 
 GPL (version 3 or later).

include/crucible/bool.h

@@ -1,13 +0,0 @@
-#ifndef CRUCIBLE_BOOL_H
-#define CRUCIBLE_BOOL_H
-
-namespace crucible {
-	struct DefaultBool {
-		bool m_b;
-		DefaultBool(bool init = false) : m_b(init) {}
-		operator bool() const { return m_b; }
-		bool &operator=(const bool &that) { return m_b = that; }
-	};
-}
-
-#endif // CRUCIBLE_BOOL_H

include/crucible/btrfs.h

@@ -130,7 +130,7 @@
 	};
 
 #endif
-	 
+
 #ifndef BTRFS_IOC_CLONE_RANGE
 
 	struct btrfs_ioctl_clone_range_args {

include/crucible/cache.h

@@ -18,7 +18,7 @@ namespace crucible {
 	public:
 		using Key = tuple<Arguments...>;
 		using Func = function<Return(Arguments...)>;
-		using Time = unsigned;
+		using Time = size_t;
 		using Value = pair<Time, Return>;
 	private:
 		Func		m_fn;
@@ -28,7 +28,7 @@ namespace crucible {
 		size_t		m_max_size;
 		mutex		m_mutex;
 
-		void check_overflow();
+		bool check_overflow();
 	public:
 		LRUCache(Func f = Func(), size_t max_size = 100);
 
@@ -52,21 +52,24 @@ namespace crucible {
 	}
 
 	template <class Return, class... Arguments>
-	void
+	bool
 	LRUCache<Return, Arguments...>::check_overflow()
 	{
-		if (m_map.size() <= m_max_size) return;
+		if (m_map.size() <= m_max_size) {
-		vector<pair<Key, Time>> map_contents;
+			return false;
-		map_contents.reserve(m_map.size());
-		for (auto i : m_map) {
-			map_contents.push_back(make_pair(i.first, i.second.first));
 		}
-		sort(map_contents.begin(), map_contents.end(), [](const pair<Key, Time> &a, const pair<Key, Time> &b) {
+		vector<pair<Key, Time>> key_times;
+		key_times.reserve(m_map.size());
+		for (auto i : m_map) {
+			key_times.push_back(make_pair(i.first, i.second.first));
+		}
+		sort(key_times.begin(), key_times.end(), [](const pair<Key, Time> &a, const pair<Key, Time> &b) {
 			return a.second < b.second;
 		});
-		for (size_t i = 0; i < map_contents.size() / 2; ++i) {
+		for (size_t i = 0; i < key_times.size() / 2; ++i) {
-			m_map.erase(map_contents[i].first);
+			m_map.erase(key_times[i].first);
 		}
+		return true;
 	}
 
 	template <class Return, class... Arguments>
@@ -141,9 +144,14 @@ namespace crucible {
 				// We hold a lock on this key so we are the ones to insert it
 				THROW_CHECK0(runtime_error, inserted);
 
-				// Release key lock and clean out overflow
+				// Release key lock, keep the cache lock
 				key_lock.unlock();
-				check_overflow();
+
+				// Check to see if we have too many items and reduce if so.
+				if (check_overflow()) {
+					// Reset iterator
+					found = m_map.find(k);
+				}
 			}
 		}
 
@@ -154,7 +162,9 @@ namespace crucible {
 		if (!inserted) {
 			found->second.first = m_ctr++;
 		}
-		return found->second.second;
+		// Make copy before releasing lock
+		auto rv = found->second.second;
+		return rv;
 	}
 
 	template<class Return, class... Arguments>
@@ -205,7 +215,12 @@ namespace crucible {
 
 				// Release key lock and clean out overflow
 				key_lock.unlock();
-				check_overflow();
+
+				// Check to see if we have too many items and reduce if so.
+				if (check_overflow()) {
+					// Reset iterator
+					found = m_map.find(k);
+				}
 			}
 		}
 

include/crucible/execpipe.h

@@ -1,28 +0,0 @@
-#ifndef CRUCIBLE_EXECPIPE_H
-#define CRUCIBLE_EXECPIPE_H
-
-#include "crucible/fd.h"
-
-#include <functional>
-#include <limits>
-#include <string>
-
-namespace crucible {
-	using namespace std;
-
-	void redirect_stdin(const Fd &child_fd);
-	void redirect_stdin_stdout(const Fd &child_fd);
-	void redirect_stdin_stdout_stderr(const Fd &child_fd);
-	void redirect_stdout(const Fd &child_fd);
-	void redirect_stdout_stderr(const Fd &child_fd);
-
-	// Open a pipe (actually socketpair) to child process, then execute code in that process.
-	// e.g. popen([] () { system("echo Hello, World!"); });
-	// Forked process will exit when function returns.
-	Fd popen(function<int()> f, function<void(const Fd &child_fd)> import_fd_fn = redirect_stdin_stdout);
-
-	// Read all the data from fd into a string
-        string read_all(Fd fd, size_t max_bytes = numeric_limits<size_t>::max(), size_t chunk_bytes = 4096);
-};
-
-#endif // CRUCIBLE_EXECPIPE_H

include/crucible/extentwalker.h

@@ -8,15 +8,15 @@ namespace crucible {
 
 	// FIXME:  ExtentCursor is probably a better name
 	struct Extent {
-		off_t		m_begin;
+		off_t		m_begin = 0;
-		off_t		m_end;
+		off_t		m_end = 0;
-		uint64_t	m_physical;
+		uint64_t	m_physical = 0;
-		uint64_t	m_flags;
+		uint64_t	m_flags = 0;
 
 		// Btrfs extent reference details
-		off_t		m_physical_len;
+		off_t		m_physical_len = 0;
-		off_t		m_logical_len;
+		off_t		m_logical_len = 0;
-		off_t		m_offset;
+		off_t		m_offset = 0;
 
 		// fiemap flags are uint32_t, so bits 32..63 are OK for us
 
@@ -38,10 +38,12 @@ namespace crucible {
 		off_t physical_len() const { return m_physical_len; }
 		off_t logical_len() const { return m_logical_len; }
 		off_t offset() const { return m_offset; }
+		bool compressed() const;
+		uint64_t bytenr() const;
 		bool operator==(const Extent &that) const;
 		bool operator!=(const Extent &that) const { return !(*this == that); }
 
-		Extent();
+		Extent() = default;
 		Extent(const Extent &e) = default;
 	};
 

include/crucible/interp.h

@@ -1,106 +0,0 @@
-#ifndef CRUCIBLE_INTERP_H
-#define CRUCIBLE_INTERP_H
-
-#include "crucible/error.h"
-
-#include <map>
-#include <memory>
-#include <string>
-#include <vector>
-
-namespace crucible {
-	using namespace std;
-
-	struct ArgList : public vector<string> {
-		ArgList(const char **argv);
-		// using vector<string>::vector ... doesn't work:
-		// error: ‘std::vector<std::basic_string<char> >::vector’ names constructor
-		// Still doesn't work in 4.9 because it can't manage a conversion
-		ArgList(const vector<string> &&that);
-	};
-
-	struct ArgActor {
-		struct ArgActorBase {
-			virtual void predicate(void *obj, string arg);
-		};
-
-		template <class T>
-		struct ArgActorDerived {
-			function<void(T, string)> m_func;
-
-			ArgActorDerived(decltype(m_func) func) :
-				m_func(func)
-			{
-			}
-
-			void predicate(void *obj, string arg) override
-			{
-				T &op = *(reinterpret_cast<T*>(obj));
-				m_func(op, obj);
-			}
-		};
-
-		template <class T>
-		ArgActor(T, function<void(T, string)> func) :
-			m_actor(make_shared(ArgActorDerived<T>(func)))
-		{
-		}
-
-		ArgActor() = default;
-
-		void predicate(void *t, string arg)
-		{
-			if (m_actor) {
-				m_actor->predicate(t, arg);
-			} else {
-				THROW_ERROR(invalid_argument, "null m_actor for predicate arg '" << arg << "'");
-			}
-		}
-
-	private:
-		shared_ptr<ArgActorBase> m_actor;
-	};
-
-	struct ArgParser {
-		~ArgParser();
-		ArgParser();
-
-		void add_opt(string opt, ArgActor actor);
-
-		template <class T>
-		void
-		parse(T t, const ArgList &args)
-		{
-			void *vt = &t;
-			parse_backend(vt, args);
-		}
-		
-	private:
-		void parse_backend(void *t, const ArgList &args);
-		map<string, ArgActor>	m_string_opts;
-	};
-
-	struct Command {
-		virtual ~Command();
-		virtual int exec(const ArgList &args) = 0;
-	};
-
-	struct Proc : public Command {
-		int exec(const ArgList &args) override;
-		Proc(const function<int(const ArgList &)> &f);
-	private:
-		function<int(const ArgList &)> m_cmd;
-	};
-
-	struct Interp {
-		virtual ~Interp();
-		Interp(const map<string, shared_ptr<Command> > &cmdlist);
-		void add_command(const string &name, const shared_ptr<Command> &command);
-		int exec(const ArgList &args);
-	private:
-		Interp(const Interp &) = delete;
-		map<string, shared_ptr<Command> > m_commands;
-	};
-
-};
-#endif // CRUCIBLE_INTERP_H

include/crucible/lockset.h

@@ -7,6 +7,7 @@
 
 #include <condition_variable>
 #include <iostream>
+#include <limits>
 #include <mutex>
 #include <set>
 
@@ -17,14 +18,18 @@ namespace crucible {
 	class LockSet {
 
 	public:
-		using key_type = T;
 		using set_type = set<T>;
+		using key_type = typename set_type::key_type;
 
 	private:
 
 		set_type			m_set;
 		mutex				m_mutex;
 		condition_variable		m_condvar;
+		size_t				m_max_size = numeric_limits<size_t>::max();
+
+		bool full();
+		bool locked(const key_type &name);
 
 	public:
 		~LockSet();
@@ -38,6 +43,8 @@ namespace crucible {
 		set_type copy();
 		void wait_unlock(double interval);
 
+		void max_size(size_t max);
+
 		class Lock {
 			LockSet		&m_lockset;
 			key_type	m_name;
@@ -68,12 +75,33 @@ namespace crucible {
 		assert(m_set.empty());
 	}
 
+	template <class T>
+	bool
+	LockSet<T>::full()
+	{
+		return m_set.size() >= m_max_size;
+	}
+
+	template <class T>
+	bool
+	LockSet<T>::locked(const key_type &name)
+	{
+		return m_set.count(name);
+	}
+
+	template <class T>
+	void
+	LockSet<T>::max_size(size_t s)
+	{
+		m_max_size = s;
+	}
+
 	template <class T>
 	void
 	LockSet<T>::lock(const key_type &name)
 	{
 		unique_lock<mutex> lock(m_mutex);
-		while (m_set.count(name)) {
+		while (full() || locked(name)) {
 			m_condvar.wait(lock);
 		}
 		auto rv = m_set.insert(name);
@@ -85,7 +113,7 @@ namespace crucible {
 	LockSet<T>::try_lock(const key_type &name)
 	{
 		unique_lock<mutex> lock(m_mutex);
-		if (m_set.count(name)) {
+		if (full() || locked(name)) {
 			return false;
 		}
 		auto rv = m_set.insert(name);
@@ -98,8 +126,8 @@ namespace crucible {
 	LockSet<T>::unlock(const key_type &name)
 	{
 		unique_lock<mutex> lock(m_mutex);
-		m_condvar.notify_all();
 		auto erase_count = m_set.erase(name);
+		m_condvar.notify_all();
 		THROW_CHECK1(invalid_argument, erase_count, erase_count == 1);
 	}
 
@@ -133,7 +161,10 @@ namespace crucible {
 	LockSet<T>::copy()
 	{
 		unique_lock<mutex> lock(m_mutex);
-		return m_set;
+		// Make temporary copy of set while protected by mutex
+		auto rv = m_set;
+		// Return temporary copy after releasing lock
+		return rv;
 	}
 
 	template <class T>

include/crucible/resource.h

@@ -3,11 +3,11 @@
 
 #include "crucible/error.h"
 
-#include <cassert>
 #include <map>
 #include <memory>
 #include <mutex>
 #include <iostream>
+#include <stdexcept>
 
 namespace crucible {
 	using namespace std;
@@ -44,36 +44,30 @@ namespace crucible {
 
 	private:
 		using traits_type = ResourceTraits<Key, Resource>;
-
+		using weak_ptr_type = weak_ptr<Resource>;
-		class ResourceHolder {
+		using map_type = map<key_type, weak_ptr_type>;
-			resource_ptr_type m_ptr;
-		public:
-			~ResourceHolder();
-			ResourceHolder(resource_ptr_type that);
-			ResourceHolder(const ResourceHolder &that) = default;
-			ResourceHolder(ResourceHolder &&that) = default;
-			ResourceHolder& operator=(ResourceHolder &&that) = default;
-			ResourceHolder& operator=(const ResourceHolder &that) = default;
-			resource_ptr_type get_resource_ptr() const;
-		};
-
-		using holder_ptr_type = shared_ptr<ResourceHolder>;
-		using weak_holder_ptr_type = weak_ptr<ResourceHolder>;
-		using map_type = map<key_type, weak_holder_ptr_type>;
 
 		// The only instance variable
-		holder_ptr_type m_ptr;
+		resource_ptr_type m_ptr;
 
 		// A bunch of static variables and functions
-		static mutex &s_mutex();
+		static mutex s_map_mutex;
-		static shared_ptr<map_type> s_map();
+		static mutex s_ptr_mutex;
-		static holder_ptr_type insert(const key_type &key);
+		static map_type s_map;
-		static holder_ptr_type insert(const resource_ptr_type &res);
+		static resource_ptr_type insert(const key_type &key);
-		static void erase(const key_type &key);
+		static resource_ptr_type insert(const resource_ptr_type &res);
+		static void clean_locked();
 		static ResourceTraits<Key, Resource> s_traits;
 
 	public:
 
+		// Exceptions
+		struct duplicate_resource : public invalid_argument {
+			key_type m_key;
+			key_type get_key() const;
+			duplicate_resource(const key_type &key);
+		};
+
 		// test for resource.  A separate operator because key_type could be confused with bool.
 		bool operator!() const;
 
@@ -89,9 +83,16 @@ namespace crucible {
 		ResourceHandle(const resource_ptr_type &res);
 		ResourceHandle& operator=(const resource_ptr_type &res);
 
-		// default constructor is public
+		// default constructor is public and mostly harmless
 		ResourceHandle() = default;
 
+		// copy/assign/move/move-assign - with a mutex to help shared_ptr be atomic
+		ResourceHandle(const ResourceHandle &that);
+		ResourceHandle(ResourceHandle &&that);
+		ResourceHandle& operator=(const ResourceHandle &that);
+		ResourceHandle& operator=(ResourceHandle &&that);
+		~ResourceHandle();
+
 		// forward anything else to the Resource constructor
 		// if we can do so unambiguously
 		template<class A1, class A2, class... Args>
@@ -109,7 +110,7 @@ namespace crucible {
 
 		// get pointer to Resource object (nothrow, result may be null)
 		resource_ptr_type get_resource_ptr() const;
-		// this version throws and is probably not thread safe
+		// this version throws
 		resource_ptr_type operator->() const;
 
 		// dynamic casting of the resource (throws if cast fails)
@@ -145,144 +146,100 @@ namespace crucible {
 	}
 
 	template <class Key, class Resource>
-	ResourceHandle<Key, Resource>::ResourceHolder::ResourceHolder(resource_ptr_type that) :
+	ResourceHandle<Key, Resource>::duplicate_resource::duplicate_resource(const key_type &key) :
-		m_ptr(that)
+		invalid_argument("duplicate resource"),
+		m_key(key)
 	{
-		// Cannot insert ourselves here since our shared_ptr does not exist yet.
 	}
 
 	template <class Key, class Resource>
-	mutex &
+	auto
-	ResourceHandle<Key, Resource>::s_mutex()
+	ResourceHandle<Key, Resource>::duplicate_resource::get_key() const -> key_type
 	{
-		static mutex gcc_won_t_instantiate_this_either;
+		return m_key;
-		return gcc_won_t_instantiate_this_either;
-	}
-
-	template <class Key, class Resource>
-	shared_ptr<typename ResourceHandle<Key, Resource>::map_type>
-	ResourceHandle<Key, Resource>::s_map()
-	{
-		static shared_ptr<map_type> gcc_won_t_instantiate_the_damn_static_vars;
-		if (!gcc_won_t_instantiate_the_damn_static_vars) {
-			gcc_won_t_instantiate_the_damn_static_vars = make_shared<map_type>();
-		}
-		return gcc_won_t_instantiate_the_damn_static_vars;
 	}
 
 	template <class Key, class Resource>
 	void
-	ResourceHandle<Key, Resource>::erase(const key_type &key)
+	ResourceHandle<Key, Resource>::clean_locked()
 	{
-		unique_lock<mutex> lock(s_mutex());
+		// Must be called with lock held
-		// Resources are allowed to set their Keys to null.
+		for (auto i = s_map.begin(); i != s_map.end(); ) {
-		if (s_traits.is_null_key(key)) {
+			auto this_i = i;
-			// Clean out any dead weak_ptr objects.
+			++i;
-			for (auto i = s_map()->begin(); i != s_map()->end(); ) {
+			if (this_i->second.expired()) {
-				if (! (*i).second.lock()) {
+				s_map.erase(this_i);
-					i = s_map()->erase(i);
-				} else {
-					++i;
-				}
 			}
-			return;
-		}
-		auto erased = s_map()->erase(key);
-		if (erased != 1) {
-			cerr << __PRETTY_FUNCTION__ << ": WARNING: s_map()->erase(" << key << ") returned " << erased << " != 1" << endl;
 		}
 	}
 
 	template <class Key, class Resource>
-	ResourceHandle<Key, Resource>::ResourceHolder::~ResourceHolder()
+	typename ResourceHandle<Key, Resource>::resource_ptr_type
-	{
-		if (!m_ptr) {
-			// Probably something harmless like a failed constructor.
-			cerr << __PRETTY_FUNCTION__ << ": WARNING: destroying null m_ptr" << endl;
-			return;
-		}
-		Key key = s_traits.get_key(*m_ptr);
-		ResourceHandle::erase(key);
-	}
-
-	template <class Key, class Resource>
-	typename ResourceHandle<Key, Resource>::holder_ptr_type
 	ResourceHandle<Key, Resource>::insert(const key_type &key)
 	{
 		// no Resources for null keys
 		if (s_traits.is_null_key(key)) {
-			return holder_ptr_type();
+			return resource_ptr_type();
 		}
-		unique_lock<mutex> lock(s_mutex());
+		unique_lock<mutex> lock(s_map_mutex);
-		// find ResourceHolder for non-null key
+		auto found = s_map.find(key);
-		auto found = s_map()->find(key);
+		if (found != s_map.end()) {
-		if (found != s_map()->end()) {
+			resource_ptr_type rv = found->second.lock();
-			holder_ptr_type rv = (*found).second.lock();
-			// a weak_ptr may have expired
 			if (rv) {
+				// Use existing Resource
 				return rv;
+			} else {
+				// It's OK for the map to temporarily contain an expired weak_ptr to some dead Resource
+				clean_locked();
 			}
 		}
 		// not found or expired, throw any existing ref away and make a new one
 		resource_ptr_type rpt = s_traits.make_resource(key);
-		holder_ptr_type hpt = make_shared<ResourceHolder>(rpt);
 		// store weak_ptr in map
-		(*s_map())[key] = hpt;
+		s_map[key] = rpt;
 		// return shared_ptr
-		return hpt;
+		return rpt;
 	};
 
 	template <class Key, class Resource>
-	typename ResourceHandle<Key, Resource>::holder_ptr_type
+	typename ResourceHandle<Key, Resource>::resource_ptr_type
 	ResourceHandle<Key, Resource>::insert(const resource_ptr_type &res)
 	{
-		// no Resource, no ResourceHolder.
+		// no Resources for null keys
 		if (!res) {
-			return holder_ptr_type();
+			return resource_ptr_type();
 		}
-		// no ResourceHolders for null keys either.
 		key_type key = s_traits.get_key(*res);
 		if (s_traits.is_null_key(key)) {
-			return holder_ptr_type();
+			return resource_ptr_type();
 		}
-		unique_lock<mutex> lock(s_mutex());
+		unique_lock<mutex> lock(s_map_mutex);
-		// find ResourceHolder for non-null key
+		// find Resource for non-null key
-		auto found = s_map()->find(key);
+		auto found = s_map.find(key);
-		if (found != s_map()->end()) {
+		if (found != s_map.end()) {
-			holder_ptr_type rv = (*found).second.lock();
+			resource_ptr_type rv = found->second.lock();
-			// The map doesn't own the ResourceHolders, the ResourceHandles do.
+			// It's OK for the map to temporarily contain an expired weak_ptr to some dead Resource...
-			// It's OK for the map to contain an expired weak_ptr to some dead ResourceHolder...
 			if (rv) {
-				// found ResourceHolder, look at pointer
+				// ...but not a duplicate Resource.
-				resource_ptr_type rp = rv->get_resource_ptr();
+				if (rv.owner_before(res) || res.owner_before(rv)) {
-				// We do not store references to null Resources.
+					throw duplicate_resource(key);
-				assert(rp);
-				// Key retrieved for an existing object must match key searched or be null.
-				key_type found_key = s_traits.get_key(*rp);
-				bool found_key_is_null = s_traits.is_null_key(found_key);
-				assert(found_key_is_null || found_key == key);
-				if (!found_key_is_null) {
-					// We do not store references to duplicate resources.
-					if (rp.owner_before(res) || res.owner_before(rp)) {
-						cerr << "inserting new Resource with existing Key " << key << " not allowed at " << __PRETTY_FUNCTION__ << endl;;
-						abort();
-						// THROW_ERROR(out_of_range, "inserting new Resource with existing Key " << key << " not allowed at " << __PRETTY_FUNCTION__);
-					}
-					// rv is good, return it
-					return rv;
 				}
+				// Use the existing Resource (discard the caller's).
+				return rv;
+			} else {
+				// Clean out expired weak_ptrs
+				clean_locked();
 			}
 		}
-		// not found or expired, make a new one
+		// not found or expired, make a new one or replace old one
-		holder_ptr_type rv = make_shared<ResourceHolder>(res);
+		s_map[key] = res;
-		s_map()->insert(make_pair(key, weak_holder_ptr_type(rv)));
+		return res;
-		// no need to check s_map result, we are either replacing a dead weak_ptr or adding a new one
-		return rv;
 	};
 
 	template <class Key, class Resource>
 	ResourceHandle<Key, Resource>::ResourceHandle(const key_type &key)
 	{
+		unique_lock<mutex> lock(s_ptr_mutex);
 		m_ptr = insert(key);
 	}
 
@@ -290,6 +247,7 @@ namespace crucible {
 	ResourceHandle<Key, Resource>&
 	ResourceHandle<Key, Resource>::operator=(const key_type &key)
 	{
+		unique_lock<mutex> lock(s_ptr_mutex);
 		m_ptr = insert(key);
 		return *this;
 	}
@@ -297,6 +255,7 @@ namespace crucible {
 	template <class Key, class Resource>
 	ResourceHandle<Key, Resource>::ResourceHandle(const resource_ptr_type &res)
 	{
+		unique_lock<mutex> lock(s_ptr_mutex);
 		m_ptr = insert(res);
 	}
 
@@ -304,36 +263,91 @@ namespace crucible {
 	ResourceHandle<Key, Resource>&
 	ResourceHandle<Key, Resource>::operator=(const resource_ptr_type &res)
 	{
+		unique_lock<mutex> lock(s_ptr_mutex);
 		m_ptr = insert(res);
 		return *this;
 	}
 
 	template <class Key, class Resource>
-	typename ResourceHandle<Key, Resource>::resource_ptr_type
+	ResourceHandle<Key, Resource>::ResourceHandle(const ResourceHandle &that)
-	ResourceHandle<Key, Resource>::ResourceHolder::get_resource_ptr() const
 	{
-		return m_ptr;
+		unique_lock<mutex> lock(s_ptr_mutex);
+		m_ptr = that.m_ptr;
+	}
+
+	template <class Key, class Resource>
+	ResourceHandle<Key, Resource>::ResourceHandle(ResourceHandle &&that)
+	{
+		unique_lock<mutex> lock(s_ptr_mutex);
+		swap(m_ptr, that.m_ptr);
+	}
+
+	template <class Key, class Resource>
+	ResourceHandle<Key, Resource> &
+	ResourceHandle<Key, Resource>::operator=(ResourceHandle &&that)
+	{
+		unique_lock<mutex> lock(s_ptr_mutex);
+		m_ptr = that.m_ptr;
+		that.m_ptr.reset();
+		return *this;
+	}
+
+	template <class Key, class Resource>
+	ResourceHandle<Key, Resource> &
+	ResourceHandle<Key, Resource>::operator=(const ResourceHandle &that)
+	{
+		unique_lock<mutex> lock(s_ptr_mutex);
+		m_ptr = that.m_ptr;
+		return *this;
+	}
+
+	template <class Key, class Resource>
+	ResourceHandle<Key, Resource>::~ResourceHandle()
+	{
+		unique_lock<mutex> lock_ptr(s_ptr_mutex);
+		// No pointer, nothing to do
+		if (!m_ptr) {
+			return;
+		}
+		// Save key so we can clean the map
+		auto key = s_traits.get_key(*m_ptr);
+		// Save pointer so we can release lock before deleting
+		auto ptr_copy = m_ptr;
+		m_ptr.reset();
+		// Release lock
+		lock_ptr.unlock();
+		// Delete our (possibly last) reference to pointer
+		ptr_copy.reset();
+		// Remove weak_ptr from map if it has expired
+		// (and not been replaced in the meantime)
+		unique_lock<mutex> lock_map(s_map_mutex);
+		auto found = s_map.find(key);
+		if (found != s_map.end() && found->second.expired()) {
+			s_map.erase(key);
+		}
 	}
 
 	template <class Key, class Resource>
 	typename ResourceHandle<Key, Resource>::resource_ptr_type
 	ResourceHandle<Key, Resource>::get_resource_ptr() const
 	{
-		if (!m_ptr) {
+		unique_lock<mutex> lock(s_ptr_mutex);
-			return resource_ptr_type();
+		// Make isolated copy of pointer with lock held, and return the copy
-		}
+		auto rv = m_ptr;
-		return m_ptr->get_resource_ptr();
+		return rv;
 	}
 
 	template <class Key, class Resource>
 	typename ResourceHandle<Key, Resource>::resource_ptr_type
 	ResourceHandle<Key, Resource>::operator->() const
 	{
-		resource_ptr_type rp = get_resource_ptr();
+		unique_lock<mutex> lock(s_ptr_mutex);
-		if (!rp) {
+		if (!m_ptr) {
 			THROW_ERROR(out_of_range, __PRETTY_FUNCTION__ << " called on null Resource");
 		}
-		return rp;
+		// Make isolated copy of pointer with lock held, and return the copy
+		auto rv = m_ptr;
+		return rv;
 	}
 
 	template <class Key, class Resource>
@@ -341,12 +355,12 @@ namespace crucible {
 	shared_ptr<T>
 	ResourceHandle<Key, Resource>::cast() const
 	{
+		unique_lock<mutex> lock(s_ptr_mutex);
 		shared_ptr<T> dp;
-		resource_ptr_type rp = get_resource_ptr();
+		if (!m_ptr) {
-		if (!rp) {
 			return dp;
 		}
-		dp = dynamic_pointer_cast<T>(rp);
+		dp = dynamic_pointer_cast<T>(m_ptr);
 		if (!dp) {
 			throw bad_cast();
 		}
@@ -357,11 +371,11 @@ namespace crucible {
 	typename ResourceHandle<Key, Resource>::key_type
 	ResourceHandle<Key, Resource>::get_key() const
 	{
-		resource_ptr_type rp = get_resource_ptr();
+		unique_lock<mutex> lock(s_ptr_mutex);
-		if (!rp) {
+		if (!m_ptr) {
 			return s_traits.get_null_key();
 		} else {
-			return s_traits.get_key(*rp);
+			return s_traits.get_key(*m_ptr);
 		}
 	}
 
@@ -378,9 +392,19 @@ namespace crucible {
 		return s_traits.is_null_key(operator key_type());
 	}
 
+	// Apparently GCC wants these to be used before they are defined.
 	template <class Key, class Resource>
 	ResourceTraits<Key, Resource> ResourceHandle<Key, Resource>::s_traits;
 
+	template <class Key, class Resource>
+	mutex ResourceHandle<Key, Resource>::s_map_mutex;
+
+	template <class Key, class Resource>
+	mutex ResourceHandle<Key, Resource>::s_ptr_mutex;
+
+	template <class Key, class Resource>
+	typename ResourceHandle<Key, Resource>::map_type ResourceHandle<Key, Resource>::s_map;
+
 
 }
 

include/crucible/time.h

@@ -32,10 +32,11 @@ namespace crucible {
 		Timer	m_timer;
 		double	m_rate;
 		double	m_burst;
-		double  m_tokens;
+		double  m_tokens = 0.0;
 		mutex	m_mutex;
 
 		void update_tokens();
+		RateLimiter() = delete;
 	public:
 		RateLimiter(double rate, double burst);
 		RateLimiter(double rate);

include/crucible/timequeue.h

@@ -77,7 +77,7 @@ namespace crucible {
 	void
 	TimeQueue<Task>::push(const Task &task, double delay)
 	{
-		Timestamp time = chrono::high_resolution_clock::now() + 
+		Timestamp time = chrono::high_resolution_clock::now() +
 			chrono::duration_cast<chrono::high_resolution_clock::duration>(chrono::duration<double>(delay));
 		unique_lock<mutex> lock(m_mutex);
 		while (m_set.size() > m_max_queue_depth) {
@@ -91,7 +91,7 @@ namespace crucible {
 	void
 	TimeQueue<Task>::push_nowait(const Task &task, double delay)
 	{
-		Timestamp time = chrono::high_resolution_clock::now() + 
+		Timestamp time = chrono::high_resolution_clock::now() +
 			chrono::duration_cast<chrono::high_resolution_clock::duration>(chrono::duration<double>(delay));
 		unique_lock<mutex> lock(m_mutex);
 		m_set.insert(Item(time, task));

include/crucible/version.h

@@ -0,0 +1,8 @@
+#ifndef CRUCIBLE_VERSION_H
+#define CRUCIBLE_VERSION_H
+
+namespace crucible {
+	extern const char *VERSION;
+}
+
+#endif CRUCIBLE_VERSION_H

include/crucible/workqueue.h

@@ -124,7 +124,9 @@ namespace crucible {
 		if (m_set.empty()) {
 			return key_type();
 		} else {
-			return *m_set.begin();
+			// Make copy with lock held
+			auto rv = *m_set.begin();
+			return rv;
 		}
 	}
 
@@ -149,7 +151,8 @@ namespace crucible {
 	WorkQueue<Task>::copy()
 	{
 		unique_lock<mutex> lock(m_mutex);
-		return m_set;
+		auto rv = m_set;
+		return rv;
 	}
 
 	template <class Task>

lib/.gitignore

@@ -0,0 +1 @@
+.version.*

lib/Makefile

@@ -4,29 +4,28 @@ OBJS = \
 	crc64.o \
 	chatter.o \
 	error.o \
-	execpipe.o \
 	extentwalker.o \
 	fd.o \
 	fs.o \
-	interp.o \
 	ntoa.o \
 	path.o \
 	process.o \
 	string.o \
 	time.o \
 	uuid.o \
+	.version.o \
 
 include ../makeflags
 
-depends.mk: *.c *.cc
+depends.mk: *.cc
-	for x in *.c; do $(CC) $(CFLAGS) -M "$$x"; done > depends.mk.new
 	for x in *.cc; do $(CXX) $(CXXFLAGS) -M "$$x"; done >> depends.mk.new
 	mv -fv depends.mk.new depends.mk
 
--include depends.mk
+.version.cc: Makefile ../makeflags *.cc ../include/crucible/*.h
+	echo "namespace crucible { const char *VERSION = \"$(shell git describe --always --dirty || echo UNKNOWN)\"; }" > .version.new.cc
+	mv -f .version.new.cc .version.cc
 
-%.o: %.c
+-include depends.mk
-	$(CC) $(CFLAGS) -o $@ -c $<
 
 %.o: %.cc ../include/crucible/%.h
 	$(CXX) $(CXXFLAGS) -o $@ -c $<

lib/execpipe.cc

@@ -1,100 +0,0 @@
-#include "crucible/execpipe.h"
-
-#include "crucible/chatter.h"
-#include "crucible/error.h"
-#include "crucible/process.h"
-
-#include <sys/types.h>
-#include <sys/socket.h>
-#include <sys/wait.h>
-#include <unistd.h>
-
-namespace crucible {
-	using namespace std;
-
-	void
-	redirect_stdin(const Fd &child_fd)
-	{
-		dup2_or_die(child_fd, STDIN_FILENO);
-	}
-
-	void
-	redirect_stdin_stdout(const Fd &child_fd)
-	{
-		dup2_or_die(child_fd, STDOUT_FILENO);
-		dup2_or_die(child_fd, STDIN_FILENO);
-	}
-
-	void
-	redirect_stdin_stdout_stderr(const Fd &child_fd)
-	{
-		dup2_or_die(child_fd, STDERR_FILENO);
-		dup2_or_die(child_fd, STDOUT_FILENO);
-		dup2_or_die(child_fd, STDIN_FILENO);
-	}
-
-	void
-	redirect_stdout_stderr(const Fd &child_fd)
-	{
-		dup2_or_die(child_fd, STDERR_FILENO);
-		dup2_or_die(child_fd, STDOUT_FILENO);
-	}
-
-	void
-	redirect_stdout(const Fd &child_fd)
-	{
-		dup2_or_die(child_fd, STDOUT_FILENO);
-	}
-
-	void
-	redirect_stderr(const Fd &child_fd)
-	{
-		dup2_or_die(child_fd, STDERR_FILENO);
-	}
-
-	Fd popen(function<int()> f, function<void(const Fd &child_fd)> import_fd_fn)
-	{
-		Fd parent_fd, child_fd;
-		{
-			pair<Fd, Fd> fd_pair = socketpair_or_die();
-			parent_fd = fd_pair.first;
-			child_fd = fd_pair.second;
-		}
-
-		pid_t fv;
-		DIE_IF_MINUS_ONE(fv = fork());
-
-		if (fv) {
-			child_fd->close();
-			return parent_fd;
-		} else {
-			int rv = EXIT_FAILURE;
-			catch_all([&]() {
-				parent_fd->close();
-				import_fd_fn(child_fd);
-
-				rv = f();
-			});
-			_exit(rv);
-		}
-	}
-
-	string
-	read_all(Fd fd, size_t max_bytes, size_t chunk_bytes)
-	{
-		char buf[chunk_bytes];
-		string str;
-		size_t rv;
-		while (1) {
-			read_partial_or_die(fd, static_cast<void *>(buf), chunk_bytes, rv);
-			if (rv == 0) {
-				break;
-			}
-			if (max_bytes - str.size() < rv) {
-				THROW_ERROR(out_of_range, "Output size limit " << max_bytes << " exceeded by appending " << rv << " bytes read to " << str.size() << " already in string");
-			}
-			str.append(buf, rv);
-		}
-		return str;
-	}
-}

lib/extentwalker.cc

@@ -79,17 +79,6 @@ namespace crucible {
 			<< "] }";
 	}
 
-	Extent::Extent() :
-		m_begin(0),
-		m_end(0),
-		m_physical(0),
-		m_flags(0),
-		m_physical_len(0),
-		m_logical_len(0),
-		m_offset(0)
-	{
-	}
-
 	Extent::operator bool() const
 	{
 		THROW_CHECK2(invalid_argument, m_begin, m_end, m_end >= m_begin);
@@ -109,6 +98,18 @@ namespace crucible {
 		return m_begin == that.m_begin && m_end == that.m_end && m_physical == that.m_physical && m_flags == that.m_flags;
 	}
 
+	bool
+	Extent::compressed() const
+	{
+		return m_flags & FIEMAP_EXTENT_ENCODED;
+	}
+
+	uint64_t
+	Extent::bytenr() const
+	{
+		return compressed() ? m_physical : m_physical - m_offset;
+	}
+
 	ExtentWalker::ExtentWalker(Fd fd) :
 		m_fd(fd),
 		m_current(m_extents.begin())
@@ -519,25 +520,26 @@ namespace crucible {
 
 			auto type = call_btrfs_get(btrfs_stack_file_extent_type, i.m_data);
 			off_t len = -1;
-                        switch (type) {
+			switch (type) {
-                                default:
+				default:
 					cerr << "Unhandled file extent type " << type << " in root " << m_tree_id << " ino " << m_stat.st_ino << endl;
 					break;
-                                case BTRFS_FILE_EXTENT_INLINE:
+				case BTRFS_FILE_EXTENT_INLINE:
 					len = ranged_cast<off_t>(call_btrfs_get(btrfs_stack_file_extent_ram_bytes, i.m_data));
 					e.m_flags |= FIEMAP_EXTENT_DATA_INLINE | FIEMAP_EXTENT_NOT_ALIGNED;
 					// Inline extents are never obscured, so don't bother filling in m_physical_len, etc.
-                                        break;
+					break;
-                                case BTRFS_FILE_EXTENT_PREALLOC:
+				case BTRFS_FILE_EXTENT_PREALLOC:
 					e.m_flags |= Extent::PREALLOC;
-                                case BTRFS_FILE_EXTENT_REG: {
+					// fallthrough
+				case BTRFS_FILE_EXTENT_REG: {
 					e.m_physical = call_btrfs_get(btrfs_stack_file_extent_disk_bytenr, i.m_data);
 
 					// This is the length of the full extent (decompressed)
-                                        off_t ram = ranged_cast<off_t>(call_btrfs_get(btrfs_stack_file_extent_ram_bytes, i.m_data));
+					off_t ram = ranged_cast<off_t>(call_btrfs_get(btrfs_stack_file_extent_ram_bytes, i.m_data));
 
 					// This is the length of the part of the extent appearing in the file (decompressed)
-                                        len = ranged_cast<off_t>(call_btrfs_get(btrfs_stack_file_extent_num_bytes, i.m_data));
+					len = ranged_cast<off_t>(call_btrfs_get(btrfs_stack_file_extent_num_bytes, i.m_data));
 
 					// This is the offset from start of on-disk extent to the part we see in the file (decompressed)
 					// May be negative due to the kind of bug we're stuck with forever, so no cast range check

lib/fs.cc

@@ -913,7 +913,7 @@ namespace crucible {
 	ostream &
 	operator<<(ostream &os, const BtrfsIoctlSearchHeader &hdr)
 	{
-		os << "BtrfsIoctlSearchHeader { " 
+		os << "BtrfsIoctlSearchHeader { "
 			<< static_cast<const btrfs_ioctl_search_header &>(hdr)
 			<< ", data = ";
 		hexdump(os, hdr.m_data);
@@ -923,7 +923,7 @@ namespace crucible {
 	ostream &
 	operator<<(ostream &os, const BtrfsIoctlSearchKey &key)
 	{
-		os << "BtrfsIoctlSearchKey { " 
+		os << "BtrfsIoctlSearchKey { "
 			<< static_cast<const btrfs_ioctl_search_key &>(key)
 			<< ", buf_size = " << key.m_buf_size
 			<< ", buf[" << key.m_result.size() << "] = {";

lib/interp.cc

@@ -1,96 +0,0 @@
-#include "crucible/interp.h"
-
-#include "crucible/chatter.h"
-
-namespace crucible {
-	using namespace std;
-
-	int
-	Proc::exec(const ArgList &args)
-	{
-		return m_cmd(args);
-	}
-
-	Proc::Proc(const function<int(const ArgList &)> &f) :
-		m_cmd(f)
-	{
-	}
-
-	Command::~Command()
-	{
-	}
-
-	ArgList::ArgList(const char **argv)
-	{
-		while (argv && *argv) {
-			push_back(*argv++);
-		}
-	}
-
-	ArgList::ArgList(const vector<string> &&that) :
-		vector<string>(that)
-	{
-	}
-
-	Interp::~Interp()
-	{
-	}
-
-	Interp::Interp(const map<string, shared_ptr<Command> > &cmdlist) :
-		m_commands(cmdlist)
-	{
-	}
-
-	void
-	Interp::add_command(const string &name, const shared_ptr<Command> &command)
-	{
-		m_commands[name] = command;
-	}
-
-	int
-	Interp::exec(const ArgList &args)
-	{
-		auto next_arg = args.begin();
-		++next_arg;
-		return m_commands.at(args[0])->exec(vector<string>(next_arg, args.end()));
-	}
-
-	ArgParser::~ArgParser()
-	{
-	}
-
-	ArgParser::ArgParser()
-	{
-	}
-
-	void
-	ArgParser::add_opt(string opt, ArgActor actor)
-	{
-		m_string_opts[opt] = actor;
-	}
-
-	void
-	ArgParser::parse_backend(void *t, const ArgList &args)
-	{
-		bool quote_args = false;
-		for (string arg : args) {
-			if (quote_args) {
-				cerr << "arg: '" << arg << "'" << endl;
-				continue;
-			}
-			if (arg == "--") {
-				quote_args = true;
-				continue;
-			}
-			if (arg.compare(0, 2, "--") == 0) {
-				auto found = m_string_opts.find(arg.substr(2, string::npos));
-				if (found != m_string_opts.end()) {
-					found->second.predicate(t, "foo");
-				}
-				(void)t;
-			}
-		}
-	}
-
-
-};

scripts/beesd

@@ -50,17 +50,20 @@ BEESHOME="${BEESHOME:-$MNT_DIR/.beeshome}"
 BEESSTATUS="${BEESSTATUS:-$WORK_DIR/$UUID.status}"
 DB_SIZE="${DB_SIZE:-$((64*AL16M))}"
 LOG_SHORT_PATH="${LOG_SHORT_PATH:-N}"
-
+LOG_FILTER_TIME="${LOG_FILTER_TIME:-N}"
-INFO "Check: BTRFS UUID exists"
-if [ ! -d "/sys/fs/btrfs/$UUID" ]; then
-    ERRO "Can't find BTRFS UUID: $UUID"
-fi
 
 INFO "Check: Disk exists"
 if [ ! -b "/dev/disk/by-uuid/$UUID" ]; then
     ERRO "Missing disk: /dev/disk/by-uuid/$UUID"
 fi
 
+it_btrfs(){ [ "$(blkid -s TYPE -o value "$1")" == "btrfs" ]; }
+
+INFO "Check: Disk with btrfs"
+if ! it_btrfs "/dev/disk/by-uuid/$UUID"; then
+    ERRO "Disk not contain btrfs: /dev/disk/by-uuid/$UUID"
+fi
+
 INFO "WORK DIR: $WORK_DIR"
 mkdir -p "$WORK_DIR" || exit 1
 
@@ -97,10 +100,29 @@ fi
     chmod 700 "$DB_PATH"
 }
 
-if YN "$LOG_SHORT_PATH"; then
+MNT_DIR="${MNT_DIR//\/\//\/}"
-    cd "$MNT_DIR" || exit 1
+
-    bees .
+filter_time(){
-else
+    if YN $LOG_FILTER_TIME; then
-    bees "$MNT_DIR"
+        sed -e 's/^.*crawl:/crawl:/g' \
-fi
+            -e 's/^.*status:/status:/g' \
+            -e 's/^.*bees:/bees:/g' \
+            -e 's/^.*crawl_writeback:/crawl_writeback:/g' \
+            -e 's/^.*main:/main:/g' \
+            -e 's/^.*hash_prefetch:/hash_prefetch:/g'
+    else
+        cat
+    fi
+}
+
+filter_path(){
+    if YN $LOG_SHORT_PATH; then
+        sed -e "s#$MNT_DIR##g"
+    else
+        cat
+    fi
+}
+
+bees "$MNT_DIR" 3>&1 2>&1 | filter_time | filter_path
+
 exit 0

scripts/beesd.conf.sample

@@ -15,6 +15,9 @@ UUID=5d3c0ad5-bedf-463d-8235-b4d4f6f99476
 ## Make path shorter in logs
 # LOG_SHORT_PATH=N
 
+## Remove timestamp from bees output
+# LOG_FILTER_TIME=N
+
 ## Bees DB size
 # Hash Table Sizing
 # sHash table entries are 16 bytes each

src/.gitignore

@@ -1 +1 @@
-bees-version.h
+bees-version.[ch]

src/Makefile

@@ -11,11 +11,13 @@ LIBS = -lcrucible -lpthread
 LDFLAGS = -L../lib -Wl,-rpath=$(shell realpath ../lib)
 
 depends.mk: Makefile *.cc
-	echo "#define BEES_VERSION \"$(shell git describe --always --dirty || echo UNKNOWN)\"" > bees-version.new.h
-	mv -f bees-version.new.h bees-version.h
 	for x in *.cc; do $(CXX) $(CXXFLAGS) -M "$$x"; done > depends.mk.new
 	mv -fv depends.mk.new depends.mk
 
+bees-version.c: Makefile *.cc *.h
+	echo "const char *BEES_VERSION = \"$(shell git describe --always --dirty || echo UNKNOWN)\";" > bees-version.new.c
+	mv -f bees-version.new.c bees-version.c
+
 -include depends.mk
 
 %.o: %.cc %.h
@@ -33,9 +35,11 @@ BEES_OBJS = \
 	bees-roots.o \
 	bees-thread.o \
 	bees-types.o \
+	bees-version.o \
 
 ../bin/bees: $(BEES_OBJS)
 	$(CXX) $(CXXFLAGS) -o "$@" $(BEES_OBJS) $(LDFLAGS) $(LIBS)
 
 clean:
-	-rm -fv *.o bees-version.h
+	-rm -fv bees-version.h
+	-rm -fv *.o bees-version.c

src/bees-context.cc

@@ -55,6 +55,12 @@ BeesFdCache::open_root(shared_ptr<BeesContext> ctx, uint64_t root)
 Fd
 BeesFdCache::open_root_ino(shared_ptr<BeesContext> ctx, uint64_t root, uint64_t ino)
 {
+	if (m_file_cache_timer.age() > BEES_COMMIT_INTERVAL) {
+		BEESINFO("Clearing open FD cache to enable file delete");
+		m_file_cache.clear();
+		m_file_cache_timer.reset();
+		BEESCOUNT(open_clear);
+	}
 	return m_file_cache(ctx, root, ino);
 }
 
@@ -969,7 +975,8 @@ BeesContext::tmpfile()
 	if (!m_tmpfiles[this_thread::get_id()]) {
 		m_tmpfiles[this_thread::get_id()] = make_shared<BeesTempFile>(shared_from_this());
 	}
-	return m_tmpfiles[this_thread::get_id()];
+	auto rv = m_tmpfiles[this_thread::get_id()];
+	return rv;
 }
 
 shared_ptr<BeesFdCache>
@@ -980,7 +987,8 @@ BeesContext::fd_cache()
 	if (!m_fd_cache) {
 		m_fd_cache = make_shared<BeesFdCache>();
 	}
-	return m_fd_cache;
+	auto rv = m_fd_cache;
+	return rv;
 }
 
 shared_ptr<BeesRoots>
@@ -991,7 +999,8 @@ BeesContext::roots()
 	if (!m_roots) {
 		m_roots = make_shared<BeesRoots>(shared_from_this());
 	}
-	return m_roots;
+	auto rv = m_roots;
+	return rv;
 }
 
 shared_ptr<BeesHashTable>
@@ -1002,7 +1011,8 @@ BeesContext::hash_table()
 	if (!m_hash_table) {
 		m_hash_table = make_shared<BeesHashTable>(shared_from_this(), "beeshash.dat");
 	}
-	return m_hash_table;
+	auto rv = m_hash_table;
+	return rv;
 }
 
 void

src/bees-hash.cc

@@ -1,4 +1,3 @@
-#include "bees-version.h"
 #include "bees.h"
 
 #include "crucible/crc64.h"
@@ -262,8 +261,8 @@ BeesHashTable::prefetch_loop()
 		graph_blob << "Uptime:  " << m_ctx->total_timer().age() << " seconds\n";
 		graph_blob << "Version: " << BEES_VERSION << "\n";
 
-		graph_blob 
+		graph_blob
-			<< "\nHash table page occupancy histogram (" << occupied_count << "/" << total_count << " cells occupied, " << (occupied_count * 100 / total_count) << "%)\n" 
+			<< "\nHash table page occupancy histogram (" << occupied_count << "/" << total_count << " cells occupied, " << (occupied_count * 100 / total_count) << "%)\n"
 			<< out.str() << "0%      |      25%      |      50%      |      75%      |   100% page fill\n"
 			<< "compressed " << compressed_count << " (" << percent(compressed_count, occupied_count) << ")"
 			<< " new-style " << compressed_offset_count << " (" << percent(compressed_offset_count, occupied_count) << ")"
@@ -309,7 +308,7 @@ BeesHashTable::fetch_missing_extent(HashType hash)
 	size_t missing_buckets = m_buckets_missing.size();
 	lock.unlock();
 
-	BEESNOTE("fetch waiting for hash extent #" << extent_number << ", " << missing_buckets << " left to fetch");
+	BEESNOTE("waiting to fetch hash extent #" << extent_number << ", " << missing_buckets << " left to fetch");
 
 	// Acquire blocking lock on this extent only
 	LockSet<uint64_t>::Lock extent_lock(m_extent_lock_set, extent_number);
@@ -336,9 +335,6 @@ BeesHashTable::fetch_missing_extent(HashType hash)
 	}
 
 	BEESCOUNT(hash_extent_in);
-	// We don't block when fetching an extent but we do slow down the
-	// prefetch thread.
-	m_prefetch_rate_limit.borrow(BLOCK_SIZE_HASHTAB_EXTENT);
 	lock.lock();
 	m_buckets_missing.erase(extent_number);
 }
@@ -597,7 +593,6 @@ BeesHashTable::BeesHashTable(shared_ptr<BeesContext> ctx, string filename, off_t
 	m_writeback_thread("hash_writeback"),
 	m_prefetch_thread("hash_prefetch"),
 	m_flush_rate_limit(BEES_FLUSH_RATE),
-	m_prefetch_rate_limit(BEES_FLUSH_RATE),
 	m_stats_file(m_ctx->home_fd(), "beesstats.txt")
 {
 	// Sanity checks to protect the implementation from its weaknesses
@@ -640,12 +635,20 @@ BeesHashTable::BeesHashTable(shared_ptr<BeesContext> ctx, string filename, off_t
 	THROW_CHECK2(runtime_error, m_void_ptr, m_bucket_ptr, m_void_ptr == m_bucket_ptr);
 	THROW_CHECK2(runtime_error, m_void_ptr, m_extent_ptr, m_void_ptr == m_extent_ptr);
 
-	{
+	// Give all the madvise hints that the kernel understands
-		// It's OK if this fails (e.g. kernel not built with CONFIG_TRANSPARENT_HUGEPAGE)
+	const struct madv_flag {
-		// We don't fork any more so DONTFORK isn't really needed
+		const char *name;
-		BEESTOOLONG("madvise(MADV_HUGEPAGE | MADV_DONTFORK)");
+		int value;
-		if (madvise(m_byte_ptr, m_size, MADV_HUGEPAGE | MADV_DONTFORK)) {
+	} madv_flags[] = {
-			BEESLOG("mostly harmless: madvise(MADV_HUGEPAGE | MADV_DONTFORK) failed: " << strerror(errno));
+		{ .name = "MADV_HUGEPAGE", .value = MADV_HUGEPAGE },
+		{ .name = "MADV_DONTFORK", .value = MADV_DONTFORK },
+		{ .name = "MADV_DONTDUMP", .value = MADV_DONTDUMP },
+		{ .name = "", .value = 0 },
+	};
+	for (auto fp = madv_flags; fp->value; ++fp) {
+		BEESTOOLONG("madvise(" << fp->name << ")");
+		if (madvise(m_byte_ptr, m_size, fp->value)) {
+			BEESLOG("madvise(..., " << fp->name << "): " << strerror(errno) << " (ignored)");
 		}
 	}
 

src/bees-resolve.cc

@@ -105,7 +105,7 @@ BeesResolver::adjust_offset(const BeesFileRange &haystack, const BeesBlockData &
 	bool is_legacy = false;
 	if (m_addr.is_compressed()) {
 		BtrfsExtentWalker ew(haystack.fd(), haystack.begin(), m_ctx->root_fd());
-		BEESTRACE("haystack extent data " << ew); 
+		BEESTRACE("haystack extent data " << ew);
 		Extent e = ew.current();
 		if (m_addr.has_compressed_offset()) {
 			off_t coff = m_addr.get_compressed_offset();

src/bees-roots.cc

@@ -768,6 +768,7 @@ BeesCrawl::fetch_extents()
 				break;
 			case BTRFS_FILE_EXTENT_PREALLOC:
 				BEESCOUNT(crawl_prealloc);
+				// fallthrough
 			case BTRFS_FILE_EXTENT_REG: {
 				auto physical = call_btrfs_get(btrfs_stack_file_extent_disk_bytenr, i.m_data);
 				auto ram = call_btrfs_get(btrfs_stack_file_extent_ram_bytes, i.m_data);
@@ -822,7 +823,8 @@ BeesCrawl::peek_front()
 	if (m_extents.empty()) {
 		return BeesFileRange();
 	}
-	return *m_extents.begin();
+	auto rv = *m_extents.begin();
+	return rv;
 }
 
 BeesFileRange
@@ -848,7 +850,8 @@ BeesCrawlState
 BeesCrawl::get_state()
 {
 	unique_lock<mutex> lock(m_state_mutex);
-	return m_state;
+	auto rv = m_state;
+	return rv;
 }
 
 void

src/bees-types.cc

@@ -286,7 +286,8 @@ Fd
 BeesFileRange::fd() const
 {
 	unique_lock<mutex> lock(s_mutex);
-	return m_fd;
+	auto rv = m_fd;
+	return rv;
 }
 
 Fd
@@ -310,7 +311,8 @@ BeesFileRange::fd(const shared_ptr<BeesContext> &ctx) const
 		}
 	}
 	// We either had a fid and opened it, or we didn't and we're just stuck with our fd
-	return m_fd;
+	auto rv = m_fd;
+	return rv;
 }
 
 BeesFileRange

src/bees.cc

@@ -1,7 +1,5 @@
-#include "bees-version.h"
 #include "bees.h"
 
-#include "crucible/interp.h"
 #include "crucible/limits.h"
 #include "crucible/process.h"
 #include "crucible/string.h"
@@ -25,7 +23,7 @@ using namespace crucible;
 using namespace std;
 
 int
-do_cmd_help(const ArgList &argv)
+do_cmd_help(const char **argv)
 {
 	cerr << "Usage: " << argv[0] << " fs-root-path [fs-root-path-2...]\n"
 		"Performs best-effort extent-same deduplication on btrfs.\n"
@@ -48,7 +46,7 @@ do_cmd_help(const ArgList &argv)
 
 RateLimiter bees_info_rate_limit(BEES_INFO_RATE, BEES_INFO_BURST);
 
-thread_local BeesTracer *BeesTracer::s_next_tracer = nullptr;
+thread_local BeesTracer *BeesTracer::tl_next_tracer = nullptr;
 
 BeesTracer::~BeesTracer()
 {
@@ -58,20 +56,20 @@ BeesTracer::~BeesTracer()
 			BEESLOG("---  END  TRACE --- exception ---");
 		}
 	}
-	s_next_tracer = m_next_tracer;
+	tl_next_tracer = m_next_tracer;
 }
 
 BeesTracer::BeesTracer(function<void()> f) :
 	m_func(f)
 {
-	m_next_tracer = s_next_tracer;
+	m_next_tracer = tl_next_tracer;
-	s_next_tracer = this;
+	tl_next_tracer = this;
 }
 
 void
 BeesTracer::trace_now()
 {
-	BeesTracer *tp = s_next_tracer;
+	BeesTracer *tp = tl_next_tracer;
 	BEESLOG("--- BEGIN TRACE ---");
 	while (tp) {
 		tp->m_func();
@@ -80,17 +78,17 @@ BeesTracer::trace_now()
 	BEESLOG("---  END  TRACE ---");
 }
 
-thread_local BeesNote *BeesNote::s_next = nullptr;
+thread_local BeesNote *BeesNote::tl_next = nullptr;
 mutex BeesNote::s_mutex;
 map<pid_t, BeesNote*> BeesNote::s_status;
-thread_local string BeesNote::s_name;
+thread_local string BeesNote::tl_name;
 
 BeesNote::~BeesNote()
 {
+	tl_next = m_prev;
 	unique_lock<mutex> lock(s_mutex);
-	s_next = m_prev;
+	if (tl_next) {
-	if (s_next) {
+		s_status[gettid()] = tl_next;
-		s_status[gettid()] = s_next;
 	} else {
 		s_status.erase(gettid());
 	}
@@ -99,28 +97,26 @@ BeesNote::~BeesNote()
 BeesNote::BeesNote(function<void(ostream &os)> f) :
 	m_func(f)
 {
+	m_name = tl_name;
+	m_prev = tl_next;
+	tl_next = this;
 	unique_lock<mutex> lock(s_mutex);
-	m_name = s_name;
+	s_status[gettid()] = tl_next;
-	m_prev = s_next;
-	s_next = this;
-	s_status[gettid()] = s_next;
 }
 
 void
 BeesNote::set_name(const string &name)
 {
-	unique_lock<mutex> lock(s_mutex);
+	tl_name = name;
-	s_name = name;
 }
 
 string
 BeesNote::get_name()
 {
-	unique_lock<mutex> lock(s_mutex);
+	if (tl_name.empty()) {
-	if (s_name.empty()) {
 		return "bees";
 	} else {
-		return s_name;
+		return tl_name;
 	}
 }
 
@@ -206,11 +202,10 @@ template <class T>
 T&
 BeesStatTmpl<T>::at(string idx)
 {
-	unique_lock<mutex> lock(m_mutex);
+	if (!m_stats_map.count(idx)) {
-    if (!m_stats_map.count(idx)) {
 		m_stats_map[idx] = 0;
 	}
-    return m_stats_map[idx];
+	return m_stats_map[idx];
 }
 
 template <class T>
@@ -218,7 +213,8 @@ T
 BeesStatTmpl<T>::at(string idx) const
 {
 	unique_lock<mutex> lock(m_mutex);
-    return m_stats_map.at(idx);
+	auto rv = m_stats_map.at(idx);
+	return rv;
 }
 
 template <class T>
@@ -226,7 +222,7 @@ void
 BeesStatTmpl<T>::add_count(string idx, size_t amount)
 {
 	unique_lock<mutex> lock(m_mutex);
-    if (!m_stats_map.count(idx)) {
+	if (!m_stats_map.count(idx)) {
 		m_stats_map[idx] = 0;
 	}
 	m_stats_map.at(idx) += amount;
@@ -258,14 +254,17 @@ BeesStats
 BeesStats::operator-(const BeesStats &that) const
 {
 	if (&that == this) return BeesStats();
+
 	unique_lock<mutex> this_lock(m_mutex);
 	BeesStats this_copy;
 	this_copy.m_stats_map = m_stats_map;
+	this_lock.unlock();
+
 	unique_lock<mutex> that_lock(that.m_mutex);
 	BeesStats that_copy;
 	that_copy.m_stats_map = that.m_stats_map;
-	this_lock.unlock();
 	that_lock.unlock();
+
 	for (auto i : that.m_stats_map) {
 		if (i.second != 0) {
 			this_copy.at(i.first) -= i.second;
@@ -462,7 +461,7 @@ BeesTempFile::BeesTempFile(shared_ptr<BeesContext> ctx) :
 
 void
 BeesTempFile::realign()
-{ 
+{
 	if (m_end_offset > BLOCK_SIZE_MAX_TEMP_FILE) {
 		BEESLOG("temporary file size " << to_hex(m_end_offset) << " > max " << BLOCK_SIZE_MAX_TEMP_FILE);
 		BEESCOUNT(tmp_trunc);
@@ -555,7 +554,7 @@ BeesTempFile::make_copy(const BeesFileRange &src)
 }
 
 int
-bees_main(ArgList args)
+bees_main(int argc, const char **argv)
 {
 	set_catch_explainer([&](string s) {
 		BEESLOG("\n\n*** EXCEPTION ***\n\t" << s << "\n***\n");
@@ -568,6 +567,9 @@ bees_main(ArgList args)
 	list<shared_ptr<BeesContext>> all_contexts;
 	shared_ptr<BeesContext> bc;
 
+	THROW_CHECK1(invalid_argument, argc, argc >= 0);
+	vector<string> args(argv + 1, argv + argc);
+
 	// Create a context and start crawlers
 	bool did_subscription = false;
 	for (string arg : args) {
@@ -603,11 +605,9 @@ main(int argc, const char **argv)
 		return 2;
 	}
 
-	ArgList args(argv + 1);
-
 	int rv = 1;
 	catch_and_explain([&]() {
-		rv = bees_main(args);
+		rv = bees_main(argc, argv);
 	});
 	return rv;
 }

src/bees.h

@@ -1,7 +1,6 @@
 #ifndef BEES_H
 #define BEES_H
 
-#include "crucible/bool.h"
 #include "crucible/cache.h"
 #include "crucible/chatter.h"
 #include "crucible/error.h"
@@ -156,12 +155,12 @@ class BeesStatTmpl {
 	map<string, T>	m_stats_map;
 	mutable mutex	m_mutex;
 
+	T& at(string idx);
 public:
 	BeesStatTmpl() = default;
 	BeesStatTmpl(const BeesStatTmpl &that);
 	BeesStatTmpl &operator=(const BeesStatTmpl &that);
 	void add_count(string idx, size_t amount = 1);
-	T& at(string idx);
 	T at(string idx) const;
 
 friend ostream& operator<< <>(ostream &os, const BeesStatTmpl<T> &bs);
@@ -185,7 +184,7 @@ class BeesTracer {
 	function<void()> m_func;
 	BeesTracer *m_next_tracer = 0;
 	
-	thread_local static BeesTracer *s_next_tracer;
+	thread_local static BeesTracer *tl_next_tracer;
 public:
 	BeesTracer(function<void()> f);
 	~BeesTracer();
@@ -201,8 +200,8 @@ class BeesNote {
 	static mutex			s_mutex;
 	static map<pid_t, BeesNote*>	s_status;
 
-	thread_local static BeesNote	*s_next;
+	thread_local static BeesNote	*tl_next;
-	thread_local static string	s_name;
+	thread_local static string	tl_name;
 
 public:
 	BeesNote(function<void(ostream &)> f);
@@ -445,7 +444,6 @@ private:
 	BeesThread  		m_writeback_thread;
 	BeesThread	        m_prefetch_thread;
 	RateLimiter		m_flush_rate_limit;
-	RateLimiter		m_prefetch_rate_limit;
 	mutex			m_extent_mutex;
 	mutex			m_bucket_mutex;
 	condition_variable	m_condvar;
@@ -487,7 +485,7 @@ class BeesCrawl {
 
 	mutex					m_mutex;
 	set<BeesFileRange>			m_extents;
-	DefaultBool				m_deferred;
+	bool					m_deferred = false;
 
 	mutex					m_state_mutex;
 	BeesCrawlState				m_state;
@@ -512,7 +510,7 @@ class BeesRoots {
 	map<uint64_t, shared_ptr<BeesCrawl>>	m_root_crawl_map;
 	mutex					m_mutex;
 	condition_variable			m_condvar;
-	DefaultBool				m_crawl_dirty;
+	bool					m_crawl_dirty = false;
 	Timer					m_crawl_timer;
 	BeesThread				m_crawl_thread;
 	BeesThread				m_writeback_thread;
@@ -568,7 +566,7 @@ class BeesBlockData {
 	mutable BeesAddress	m_addr;
 	mutable Blob		m_data;
 	mutable BeesHash	m_hash;
-	mutable DefaultBool	m_hash_done;
+	mutable bool		m_hash_done = false;
 
 public:
 	// Constructor with the immutable fields
@@ -607,7 +605,7 @@ friend ostream & operator<<(ostream &os, const BeesRangePair &brp);
 };
 
 class BeesWorkQueueBase {
-	string 				m_name; 
+	string 				m_name;
 
 protected:
 	static mutex			s_mutex;
@@ -622,7 +620,7 @@ public:
 
 	virtual size_t active_size() const = 0;
 	virtual list<string> peek_active(size_t count) const = 0;
- 
+
 	static void for_each_work_queue(function<void(BeesWorkQueueBase *)> f);
 };
 
@@ -661,6 +659,7 @@ class BeesFdCache {
 	LRUCache<Fd, shared_ptr<BeesContext>, uint64_t>			m_root_cache;
 	LRUCache<Fd, shared_ptr<BeesContext>, uint64_t, uint64_t>	m_file_cache;
 	Timer								m_root_cache_timer;
+	Timer								m_file_cache_timer;
 
 public:
 	BeesFdCache();
@@ -672,7 +671,7 @@ public:
 struct BeesResolveAddrResult {
 	BeesResolveAddrResult();
 	vector<BtrfsInodeOffsetRoot> m_biors;
-	DefaultBool m_is_toxic;
+	bool m_is_toxic = false;
 	bool is_toxic() const { return m_is_toxic; }
 };
 
@@ -750,22 +749,22 @@ class BeesResolver {
 	unsigned				m_bior_count;
 
 	// We found matching data, so we can dedup
-	DefaultBool				m_found_data;
+	bool					m_found_data = false;
 
 	// We found matching data, so we *did* dedup
-	DefaultBool				m_found_dup;
+	bool					m_found_dup = false;
 
 	// We found matching hash, so the hash table is still correct
-	DefaultBool				m_found_hash;
+	bool					m_found_hash = false;
 
 	// We found matching physical address, so the hash table isn't totally wrong
-	DefaultBool				m_found_addr;
+	bool					m_found_addr = false;
 
 	// We found matching physical address, but data did not match
-	DefaultBool				m_wrong_data;
+	bool					m_wrong_data = false;
 
 	// The whole thing is a placebo to avoid crippling btrfs performance bugs
-	DefaultBool				m_is_toxic;
+	bool					m_is_toxic = false;
 
 	BeesFileRange chase_extent_ref(const BtrfsInodeOffsetRoot &bior, BeesBlockData &needle_bbd);
 	BeesBlockData adjust_offset(const BeesFileRange &haystack, const BeesBlockData &needle);
@@ -819,6 +818,7 @@ public:
 };
 
 // And now, a giant pile of extern declarations
+extern const char *BEES_VERSION;
 string pretty(double d);
 extern RateLimiter bees_info_rate_limit;
 void bees_sync(int fd);

test/Makefile

@@ -1,9 +1,7 @@
 PROGRAMS = \
 	chatter \
 	crc64 \
-	execpipe \
 	fd \
-	interp \
 	limits \
 	path \
 	process \
@@ -21,7 +19,7 @@ LDFLAGS = -L../lib -Wl,-rpath=$(shell realpath ../lib)
 depends.mk: *.cc
 	for x in *.cc; do $(CXX) $(CXXFLAGS) -M "$$x"; done >> depends.mk.new
 	mv -fv depends.mk.new depends.mk
-                                       
+
 -include depends.mk
 
 %.o: %.cc %.h ../makeflags

test/execpipe.cc

@@ -1,64 +0,0 @@
-#include "tests.h"
-
-#include "crucible/execpipe.h"
-
-#include <ios>
-#include <cassert>
-#include <cstring>
-#include <cstdlib>
-#include <stdexcept>
-
-#include <unistd.h>
-
-using namespace crucible;
-using namespace std;
-
-#if 1 // Needs rework
-static inline
-void
-test_hello_world()
-{
-	// alarm(9);
-	Fd fd = popen([]() { return system("echo Hello, World!"); });
-	char buf[1024];
-	size_t rv = -1;
-	read_partial_or_die(fd, buf, rv);
-	assert(rv > 0);
-	string b(buf, buf + rv - 1);
-	// cerr << "hello_world says: '" << b << "'" << endl;
-	assert(b == "Hello, World!");
-}
-
-static inline
-void
-test_read_limit(size_t limit = 4096)
-{
-	alarm(9);
-	Fd fd = popen([]() { return system("yes Hello!"); });
-	try {
-		string b = read_all(fd, limit);
-	} catch (out_of_range &re) {
-		return;
-	}
-	assert(!"no exception thrown by read_all");
-}
-#endif
-
-namespace crucible {
-	extern bool assert_no_leaked_fds();
-};
-
-int
-main(int, char**)
-{
-#if 1
-	RUN_A_TEST(test_hello_world());
-	assert(assert_no_leaked_fds());
-	RUN_A_TEST(test_read_limit(4095));
-	RUN_A_TEST(test_read_limit(4096));
-	RUN_A_TEST(test_read_limit(4097));
-	assert(assert_no_leaked_fds());
-#endif
-
-	exit(EXIT_SUCCESS);
-}

test/interp.cc

@@ -1,88 +0,0 @@
-#include "tests.h"
-
-#include "crucible/interp.h"
-
-using namespace crucible;
-using namespace std;
-
-/***********************************************************************
-
-How this should work:
-
-Interpreter reads an arg list:
-
-	argv[0] --method0args --method1arg arg1 --method1arg=arg1 -- args...
-
-argv[0] should look up a shared_ptr<Command> which creates an object of
-type shared_ptr<Process>.  This object is used to receive args by
-method calls or one at a time.
-
-<Command> and <Process> can be the same object, or not.
-
-Process p methods:
-
-	p->spawn(Interp*) -> Process
-	p->exec(ArgList) -> Process / Result
-	p->method (from ArgParser<>)
-		p->finish() -> void (destroys object without early destruction warnings...?)
-		p->~Process() -> complains loudly if finish() not called first...?
-
-Result might be a pair of Process, string.  Or just string.
-
-ArgParser should be more like GetOpt:
-
-	build a dictionary and an arg list from arguments
-	Process methods should interrogate ArgParser
-	ArgParser might have a table of boolean and string option names so it can reject invalid options
-		but if it had that, we could also pass in Process and have it call methods on it
-		...but that is a _lot_ of pointer-hiding when we could KISS
-		...but if we had that solved, argparser tables look like lists of method names
-	ArgParser<T> has a table of names and methods on object of type T
-		ArgParser hides everything behind void* and hands off to a compiled implementation to do callbacks
-
-Extreme simplification:  arguments are themselves executable
-
-	so '--method_foo arg' really means construct MethodFoo(arg) and cast to shared_ptr<ProcArg>
-	then Process->invokeSomething(ProcArg)
-	too extreme, use argparser instead
-
-***********************************************************************/
-
-void
-test_arg_parser()
-{
-	ArgParser ap;
-	ArgList al( { "abc", "--def", "ghi" } );
-	ap.parse(NULL, al);
-}
-
-struct Thing {
-	int m_i;
-	double m_d;
-	string m_s;
-
-	void set_i(int i) { cerr << "i = " << i << endl; m_i = i; }
-	void set_d(double d) { cerr << "d = " << d << endl; m_d = d; }
-	void set_s(string s) { cerr << "s = " << s << endl; m_s = s; }
-};
-
-template <typename F, typename T, typename A>
-void
-assign(T& t, F f, A a)
-{
-	cerr << __PRETTY_FUNCTION__ << " - a = " << a << endl;
-	(t.*f)(a);
-}
-
-int
-main(int, char**)
-{
-	RUN_A_TEST(test_arg_parser());
-
-	Thing p;
-	assign(p, &Thing::set_i, 5);
-
-	cerr << "p.m_i = " << p.m_i << endl;
-
-	exit(EXIT_SUCCESS);
-}