Ehooks comments and cleanup.

include/jemalloc/internal/ehooks.h

@@ -1,6 +1,13 @@
 #ifndef JEMALLOC_INTERNAL_EHOOKS_H
 #define JEMALLOC_INTERNAL_EHOOKS_H
 
+/*
+ * This module is the internal interface to the extent hooks (both
+ * user-specified and external).  Eventually, this will give us the flexibility
+ * to use multiple different versions of user-visible extent-hook APIs under a
+ * single user interface.
+ */
+
 #include "jemalloc/internal/atomic.h"
 
 extern const extent_hooks_t ehooks_default_extent_hooks;
@@ -13,41 +20,45 @@ struct ehooks_s {
 
 extern const extent_hooks_t ehooks_default_extent_hooks;
 
-/* NOT PUBLIC. */
+/*
+ * These are not really part of the public API.  Each hook has a fast-path for
+ * the default-hooks case that can avoid various small inefficiencies:
+ *   - Forgetting tsd and then calling tsd_get within the hook.
+ *   - Getting more state than necessary out of the extent_t.
+ *   - Doing arena_ind -> arena -> arena_ind lookups.
+ * By making the calls to these functions visible to the compiler, it can move
+ * those extra bits of computation down below the fast-paths where they get ignored.
+ */
 void *ehooks_default_alloc_impl(tsdn_t *tsdn, void *new_addr, size_t size,
     size_t alignment, bool *zero, bool *commit, unsigned arena_ind);
-void *ehooks_default_alloc(extent_hooks_t *extent_hooks, void *new_addr,
-    size_t size, size_t alignment, bool *zero, bool *commit,
-    unsigned arena_ind);
 bool ehooks_default_dalloc_impl(void *addr, size_t size);
-bool ehooks_default_dalloc(extent_hooks_t *extent_hooks, void *addr,
-    size_t size, bool committed, unsigned arena_ind);
 void ehooks_default_destroy_impl(void *addr, size_t size);
-void ehooks_default_destroy(extent_hooks_t *extent_hooks, void *addr,
-    size_t size, bool committed, unsigned arena_ind);
 bool ehooks_default_commit_impl(void *addr, size_t offset, size_t length);
-bool ehooks_default_commit(extent_hooks_t *extent_hooks, void *addr, size_t size,
-    size_t offset, size_t length, unsigned arena_ind);
 bool ehooks_default_decommit_impl(void *addr, size_t offset, size_t length);
-bool ehooks_default_decommit(extent_hooks_t *extent_hooks, void *addr, size_t size,
-    size_t offset, size_t length, unsigned arena_ind);
 #ifdef PAGES_CAN_PURGE_LAZY
 bool ehooks_default_purge_lazy_impl(void *addr, size_t offset, size_t length);
-bool ehooks_default_purge_lazy(extent_hooks_t *extent_hooks, void *addr, size_t size,
-    size_t offset, size_t length, unsigned arena_ind);
 #endif
 #ifdef PAGES_CAN_PURGE_FORCED
 bool ehooks_default_purge_forced_impl(void *addr, size_t offset, size_t length);
-bool ehooks_default_purge_forced(extent_hooks_t *extent_hooks, void *addr,
-    size_t size, size_t offset, size_t length, unsigned arena_ind);
 #endif
 bool ehooks_default_split_impl();
-bool ehooks_default_split(extent_hooks_t *extent_hooks, void *addr, size_t size,
-    size_t size_a, size_t size_b, bool committed, unsigned arena_ind);
 bool ehooks_default_merge_impl(void *addr_a, void *addr_b);
-bool ehooks_default_merge(extent_hooks_t *extent_hooks, void *addr_a, size_t size_a,
-    void *addr_b, size_t size_b, bool committed, unsigned arena_ind);
 
+/*
+ * We don't officially support reentrancy from wtihin the extent hooks.  But
+ * various people who sit within throwing distance of the jemalloc team want
+ * that functionality in certain limited cases.  The default reentrancy guards
+ * assert that we're not reentrant from a0 (since it's the bootstrap arena,
+ * where reentrant allocations would be redirected), which we would incorrectly
+ * trigger in cases where a0 has extent hooks (those hooks themselves can't be
+ * reentrant, then, but there are reasonable uses for such functionality, like
+ * putting internal metadata on hugepages).  Therefore, we use the raw
+ * reentrancy guards.
+ *
+ * Eventually, we need to think more carefully about whether and where we
+ * support allocating from within extent hooks (and what that means for things
+ * like profiling, stats collection, etc.), and document what the guarantee is.
+ */
 static inline void
 ehooks_pre_reentrancy(tsdn_t *tsdn) {
 	tsd_t *tsd = tsdn_null(tsdn) ? tsd_fetch() : tsdn_tsd(tsdn);
@@ -60,7 +71,7 @@ ehooks_post_reentrancy(tsdn_t *tsdn) {
 	tsd_post_reentrancy_raw(tsd);
 }
 
-/* PUBLIC. */
+/* Beginning of the public API. */
 void ehooks_init(ehooks_t *ehooks, extent_hooks_t *extent_hooks);
 
 static inline void
@@ -79,21 +90,11 @@ ehooks_are_default(ehooks_t *ehooks) {
 	    &ehooks_default_extent_hooks;
 }
 
-static inline bool
-ehooks_destroy_is_noop(ehooks_t *ehooks) {
-	return ehooks_get_extent_hooks_ptr(ehooks)->destroy == NULL;
-}
-
-static inline bool
-ehooks_purge_lazy_will_fail(ehooks_t *ehooks) {
-	return ehooks_get_extent_hooks_ptr(ehooks)->purge_lazy == NULL;
-}
-
-static inline bool
-ehooks_purge_forced_will_fail(ehooks_t *ehooks) {
-	return ehooks_get_extent_hooks_ptr(ehooks)->purge_forced == NULL;
-}
-
+/*
+ * In some cases, a caller needs to allocate resources before attempting to call
+ * a hook.  If that hook is doomed to fail, this is wasteful.  We therefore
+ * include some checks for such cases.
+ */
 static inline bool
 ehooks_split_will_fail(ehooks_t *ehooks) {
 	return ehooks_get_extent_hooks_ptr(ehooks)->split == NULL;

src/ehooks.c

@@ -57,7 +57,7 @@ ehooks_default_alloc_impl(tsdn_t *tsdn, void *new_addr, size_t size,
 	return ret;
 }
 
-void *
+static void *
 ehooks_default_alloc(extent_hooks_t *extent_hooks, void *new_addr, size_t size,
     size_t alignment, bool *zero, bool *commit, unsigned arena_ind) {
 	tsdn_t *tsdn;
@@ -84,7 +84,7 @@ ehooks_default_dalloc_impl(void *addr, size_t size) {
 	return true;
 }
 
-bool
+static bool
 ehooks_default_dalloc(extent_hooks_t *extent_hooks, void *addr, size_t size,
     bool committed, unsigned arena_ind) {
 	return ehooks_default_dalloc_impl(addr, size);
@@ -97,7 +97,7 @@ ehooks_default_destroy_impl(void *addr, size_t size) {
 	}
 }
 
-void
+static void
 ehooks_default_destroy(extent_hooks_t *extent_hooks, void *addr, size_t size,
     bool committed, unsigned arena_ind) {
 	ehooks_default_destroy_impl(addr, size);
@@ -109,7 +109,7 @@ ehooks_default_commit_impl(void *addr, size_t offset, size_t length) {
 	    length);
 }
 
-bool
+static bool
 ehooks_default_commit(extent_hooks_t *extent_hooks, void *addr, size_t size,
     size_t offset, size_t length, unsigned arena_ind) {
 	return ehooks_default_commit_impl(addr, offset, length);
@@ -121,7 +121,7 @@ ehooks_default_decommit_impl(void *addr, size_t offset, size_t length) {
 	    length);
 }
 
-bool
+static bool
 ehooks_default_decommit(extent_hooks_t *extent_hooks, void *addr, size_t size,
     size_t offset, size_t length, unsigned arena_ind) {
 	return ehooks_default_decommit_impl(addr, offset, length);
@@ -134,7 +134,7 @@ ehooks_default_purge_lazy_impl(void *addr, size_t offset, size_t length) {
 	    length);
 }
 
-bool
+static bool
 ehooks_default_purge_lazy(extent_hooks_t *extent_hooks, void *addr, size_t size,
     size_t offset, size_t length, unsigned arena_ind) {
 	assert(addr != NULL);
@@ -152,7 +152,7 @@ ehooks_default_purge_forced_impl(void *addr, size_t offset, size_t length) {
 	    (uintptr_t)offset), length);
 }
 
-bool
+static bool
 ehooks_default_purge_forced(extent_hooks_t *extent_hooks, void *addr,
     size_t size, size_t offset, size_t length, unsigned arena_ind) {
 	assert(addr != NULL);
@@ -177,7 +177,7 @@ ehooks_default_split_impl() {
 	return false;
 }
 
-bool
+static bool
 ehooks_default_split(extent_hooks_t *extent_hooks, void *addr, size_t size,
     size_t size_a, size_t size_b, bool committed, unsigned arena_ind) {
 	return ehooks_default_split_impl();
@@ -195,7 +195,7 @@ ehooks_default_merge_impl(void *addr_a, void *addr_b) {
 	return false;
 }
 
-bool
+static bool
 ehooks_default_merge(extent_hooks_t *extent_hooks, void *addr_a, size_t size_a,
     void *addr_b, size_t size_b, bool committed, unsigned arena_ind) {
 	if (!maps_coalesce) {