1 files changed, 183 insertions, 134 deletions

diff --git a/erts/emulator/beam/erl_nif.c b/erts/emulator/beam/erl_nif.c
index 94ebf88b56..d2000aa71e 100644
--- a/erts/emulator/beam/erl_nif.c
+++ b/erts/emulator/beam/erl_nif.c
@@ -237,9 +237,11 @@ static void cache_env(ErlNifEnv* env);
 static void full_flush_env(ErlNifEnv *env);
 static void flush_env(ErlNifEnv* env);
 
-/* Temporary object header, auto-deallocated when NIF returns
- * or when independent environment is cleared.
- */
+/* Temporary object header, auto-deallocated when NIF returns or when
+ * independent environment is cleared.
+ *
+ * The payload can be accessed with &tmp_obj_ptr[1] but keep in mind that its
+ * first element must not require greater alignment than `next`. */
 struct enif_tmp_obj_t {
     struct enif_tmp_obj_t* next;
     void (*dtor)(struct enif_tmp_obj_t*);
@@ -256,6 +258,46 @@ static ERTS_INLINE void free_tmp_objs(ErlNifEnv* env)
     }
 }
 
+/* Whether the given environment is bound to a process and will be cleaned up
+ * when the NIF returns. It's safe to use temp_alloc for objects in
+ * env->tmp_obj_list when this is true. */
+static ERTS_INLINE int is_proc_bound(ErlNifEnv *env)
+{
+    return env->mod_nif != NULL;
+}
+
+/* Allocates and attaches an object to the given environment, running its
+ * destructor when the environment is cleared. To avoid temporary variables the
+ * address of the allocated object is returned instead of the enif_tmp_obj_t.
+ *
+ * The destructor *must* call `erts_free(tmp_obj->allocator, tmp_obj)` to free
+ * the object. If the destructor needs to refer to the allocated object its
+ * address will be &tmp_obj[1]. */
+static ERTS_INLINE void *alloc_tmp_obj(ErlNifEnv *env, size_t size,
+                                       void (*dtor)(struct enif_tmp_obj_t*)) {
+    struct enif_tmp_obj_t *tmp_obj;
+    ErtsAlcType_t allocator;
+
+    allocator = is_proc_bound(env) ? ERTS_ALC_T_TMP : ERTS_ALC_T_NIF;
+
+    tmp_obj = erts_alloc(allocator, sizeof(struct enif_tmp_obj_t) + MAX(1, size));
+
+    tmp_obj->next = env->tmp_obj_list;
+    tmp_obj->allocator = allocator;
+    tmp_obj->dtor = dtor;
+
+    env->tmp_obj_list = tmp_obj;
+
+    return (void*)&tmp_obj[1];
+}
+
+/* Generic destructor for objects allocated through alloc_tmp_obj that don't
+ * care about their payload. */
+static void tmp_alloc_dtor(struct enif_tmp_obj_t *tmp_obj)
+{
+    erts_free(tmp_obj->allocator, tmp_obj);
+}
+
 void erts_post_nif(ErlNifEnv* env)
 {
     erts_unblock_fpe(env->fpe_was_unmasked);
@@ -446,6 +488,7 @@ static void cache_env(ErlNifEnv* env)
 	env->hp_end = env->heap_frag->mem + env->heap_frag->alloc_size;
     }
 }
+
 void* enif_priv_data(ErlNifEnv* env)
 {
     return env->mod_nif->priv_data;
@@ -1019,11 +1062,6 @@ int enif_is_number(ErlNifEnv* env, ERL_NIF_TERM term)
     return is_number(term);
 }
 
-static ERTS_INLINE int is_proc_bound(ErlNifEnv* env)
-{
-    return env->mod_nif != NULL;
-}
-
 static void aligned_binary_dtor(struct enif_tmp_obj_t* obj)
 {
     erts_free_aligned_binary_bytes_extra((byte*)obj, obj->allocator);
@@ -1058,22 +1096,14 @@ int enif_inspect_binary(ErlNifEnv* env, Eterm bin_term, ErlNifBinary* bin)
 	u.tmp->dtor = &aligned_binary_dtor;
 	env->tmp_obj_list = u.tmp;
     }
-    bin->bin_term = bin_term;
     bin->size = binary_size(bin_term);
     bin->ref_bin = NULL;
     ADD_READONLY_CHECK(env, bin->data, bin->size);
     return 1;
 }
 
-static void tmp_alloc_dtor(struct enif_tmp_obj_t* obj)
-{
-    erts_free(obj->allocator,  obj);
-}
-
 int enif_inspect_iolist_as_binary(ErlNifEnv* env, Eterm term, ErlNifBinary* bin)
 {
-    struct enif_tmp_obj_t* tobj;
-    ErtsAlcType_t allocator;
     ErlDrvSizeT sz;
     if (is_binary(term)) {
 	return enif_inspect_binary(env,term,bin);
@@ -1081,7 +1111,6 @@ int enif_inspect_iolist_as_binary(ErlNifEnv* env, Eterm term, ErlNifBinary* bin)
     if (is_nil(term)) {
 	bin->data = (unsigned char*) &bin->data; /* dummy non-NULL */
 	bin->size = 0;
-	bin->bin_term = THE_NON_VALUE;
 	bin->ref_bin = NULL;
 	return 1;
     }
@@ -1089,16 +1118,8 @@ int enif_inspect_iolist_as_binary(ErlNifEnv* env, Eterm term, ErlNifBinary* bin)
 	return 0;
     }
 
-    allocator = is_proc_bound(env) ? ERTS_ALC_T_TMP : ERTS_ALC_T_NIF;
-    tobj = erts_alloc(allocator, sz + sizeof(struct enif_tmp_obj_t));
-    tobj->allocator = allocator;
-    tobj->next = env->tmp_obj_list;
-    tobj->dtor = &tmp_alloc_dtor;
-    env->tmp_obj_list = tobj;
-
-    bin->data = (unsigned char*) &tobj[1]; 
+    bin->data = alloc_tmp_obj(env, sz, &tmp_alloc_dtor);
     bin->size = sz;
-    bin->bin_term = THE_NON_VALUE;
     bin->ref_bin = NULL;
     erts_iolist_to_buf(term, (char*) bin->data, sz);
     ADD_READONLY_CHECK(env, bin->data, bin->size); 
@@ -1116,7 +1137,6 @@ int enif_alloc_binary(size_t size, ErlNifBinary* bin)
 
     bin->size = size;
     bin->data = (unsigned char*) refbin->orig_bytes;
-    bin->bin_term = THE_NON_VALUE;
     bin->ref_bin = refbin;
     return 1;
 }
@@ -1150,12 +1170,10 @@ void enif_release_binary(ErlNifBinary* bin)
 {
     if (bin->ref_bin != NULL) {
 	Binary* refbin = bin->ref_bin;
-	ASSERT(bin->bin_term == THE_NON_VALUE);
         erts_bin_release(refbin);
     }
 #ifdef DEBUG
     bin->data = NULL;
-    bin->bin_term = THE_NON_VALUE;
     bin->ref_bin = NULL;
 #endif
 }
@@ -1312,29 +1330,51 @@ int enif_get_string(ErlNifEnv *env, ERL_NIF_TERM list, char* buf, unsigned len,
 
 Eterm enif_make_binary(ErlNifEnv* env, ErlNifBinary* bin)
 {
-    if (bin->bin_term != THE_NON_VALUE) {
-	return bin->bin_term;
-    }
-    else if (bin->ref_bin != NULL) {
-	Binary* bptr = bin->ref_bin;
-	Eterm bin_term;
-	
-        bin_term = erts_build_proc_bin(&MSO(env->proc),
-                                       alloc_heap(env, PROC_BIN_SIZE),
-                                       bptr);
-	if (erts_refc_read(&bptr->intern.refc, 1) == 1) {
-	    /* Total ownership transfer */
-	    bin->ref_bin = NULL;
-	    bin->bin_term = bin_term;
-	}
-	return bin_term;
-    }
-    else {
-	flush_env(env);
-	bin->bin_term = new_binary(env->proc, bin->data, bin->size);
-	cache_env(env);
-	return bin->bin_term;
+    Eterm bin_term;
+
+    if (bin->ref_bin != NULL) {
+        Binary* binary = bin->ref_bin;
+
+        /* If the binary is smaller than the heap binary limit we'll return a
+         * heap binary to reduce the number of small refc binaries in the
+         * system. We can't simply release the refc binary right away however;
+         * the documentation states that the binary should be considered
+         * read-only from this point on, which implies that it should still be
+         * readable.
+         *
+         * We could keep it alive until we return by adding it to the temporary
+         * object list, but that requires an off-heap allocation which is
+         * potentially quite slow, so we create a dummy ProcBin instead and
+         * rely on the next minor GC to get rid of it. */
+        if (bin->size <= ERL_ONHEAP_BIN_LIMIT) {
+            ErlHeapBin* hb;
+
+            hb = (ErlHeapBin*)alloc_heap(env, heap_bin_size(bin->size));
+            hb->thing_word = header_heap_bin(bin->size);
+            hb->size = bin->size;
+
+            sys_memcpy(hb->data, bin->data, bin->size);
+
+            erts_build_proc_bin(&MSO(env->proc),
+                                alloc_heap(env, PROC_BIN_SIZE),
+                                binary);
+
+            bin_term = make_binary(hb);
+        } else {
+            bin_term = erts_build_proc_bin(&MSO(env->proc),
+                                           alloc_heap(env, PROC_BIN_SIZE),
+                                           binary);
+        }
+
+        /* Our (possibly shared) ownership has been transferred to the term. */
+        bin->ref_bin = NULL;
+    } else {
+        flush_env(env);
+        bin_term = new_binary(env->proc, bin->data, bin->size);
+        cache_env(env);
     }
+
+    return bin_term;
 }
 
 Eterm enif_make_sub_binary(ErlNifEnv* env, ERL_NIF_TERM bin_term,
@@ -3284,8 +3324,8 @@ typedef struct {
     Eterm sublist_start;
     Eterm sublist_end;
 
-    UWord offheap_size;
-    UWord onheap_size;
+    UWord referenced_size;
+    UWord copied_size;
 
     UWord iovec_len;
 } iovec_slice_t;
@@ -3295,16 +3335,16 @@ static int examine_iovec_term(Eterm list, UWord max_length, iovec_slice_t *resul
 
     result->sublist_start = list;
     result->sublist_length = 0;
-    result->offheap_size = 0;
-    result->onheap_size = 0;
+    result->referenced_size = 0;
+    result->copied_size = 0;
     result->iovec_len = 0;
 
     lookahead = result->sublist_start;
 
     while (is_list(lookahead)) {
-        Eterm *binary_header, binary;
+        UWord byte_size;
+        Eterm binary;
         Eterm *cell;
-        UWord size;
 
         cell = list_val(lookahead);
         binary = CAR(cell);
@@ -3313,35 +3353,36 @@ static int examine_iovec_term(Eterm list, UWord max_length, iovec_slice_t *resul
             return 0;
         }
 
-        size = binary_size(binary);
-        binary_header = binary_val(binary);
+        byte_size = binary_size(binary);
+
+        if (byte_size > 0) {
+            int bit_offset, bit_size;
+            Eterm parent_binary;
+            UWord byte_offset;
 
-        if (size > 0) {
-            /* If we're a sub-binary we'll need to check our underlying binary
-             * to determine whether we're on-heap or not. */
-            if (thing_subtag(*binary_header) == SUB_BINARY_SUBTAG) {
-                ErlSubBin *sb = (ErlSubBin*)binary_header;
+            int requires_copying;
 
-                /* Reject bitstrings */
-                if((sb->bitoffs + sb->bitsize) > 0) {
-                    return 0;
-                }
+            ERTS_GET_REAL_BIN(binary, parent_binary, byte_offset,
+                bit_offset, bit_size);
 
-                ASSERT(size <= binary_size(sb->orig));
-                binary_header = binary_val(sb->orig);
+            (void)byte_offset;
+
+            if (bit_size != 0) {
+                return 0;
             }
 
-            if (thing_subtag(*binary_header) == HEAP_BINARY_SUBTAG) {
-                ASSERT(size <= ERL_ONHEAP_BIN_LIMIT);
+            /* If we're unaligned or an on-heap binary we'll need to copy
+             * ourselves over to a temporary buffer. */
+            requires_copying = (bit_offset != 0) ||
+                thing_subtag(*binary_val(parent_binary)) == HEAP_BINARY_SUBTAG;
 
-                result->iovec_len += 1;
-                result->onheap_size += size;
+            if (requires_copying) {
+                result->copied_size += byte_size;
             } else {
-                ASSERT(thing_subtag(*binary_header) == REFC_BINARY_SUBTAG);
-
-                result->iovec_len += 1 + size / MAX_SYSIOVEC_IOVLEN;
-                result->offheap_size += size;
+                result->referenced_size += byte_size;
             }
+
+            result->iovec_len += 1 + byte_size / MAX_SYSIOVEC_IOVLEN;
         }
 
         result->sublist_length += 1;
@@ -3361,7 +3402,9 @@ static int examine_iovec_term(Eterm list, UWord max_length, iovec_slice_t *resul
     return 1;
 }
 
-static void inspect_raw_binary_data(Eterm binary, ErlNifBinary *result) {
+static void marshal_iovec_binary(Eterm binary, ErlNifBinary *copy_buffer,
+        UWord *copy_offset, ErlNifBinary *result) {
+
     Eterm *parent_header;
     Eterm parent_binary;
 
@@ -3372,10 +3415,11 @@ static void inspect_raw_binary_data(Eterm binary, ErlNifBinary *result) {
 
     ERTS_GET_REAL_BIN(binary, parent_binary, byte_offset, bit_offset, bit_size);
 
+    ASSERT(bit_size == 0);
+
     parent_header = binary_val(parent_binary);
 
     result->size = binary_size(binary);
-    result->bin_term = binary;
 
     if (thing_subtag(*parent_header) == REFC_BINARY_SUBTAG) {
         ProcBin *pb = (ProcBin*)parent_header;
@@ -3398,24 +3442,48 @@ static void inspect_raw_binary_data(Eterm binary, ErlNifBinary *result) {
         result->data = &((unsigned char*)&hb->data)[byte_offset];
         result->ref_bin = NULL;
     }
+
+    /* If this isn't an *aligned* refc binary, copy its contents to the buffer
+     * and reference that instead. */
+
+    if (result->ref_bin == NULL || bit_offset != 0) {
+        ASSERT(copy_buffer->ref_bin != NULL && copy_buffer->data != NULL);
+        ASSERT(result->size <= (copy_buffer->size - *copy_offset));
+
+        if (bit_offset == 0) {
+            sys_memcpy(&copy_buffer->data[*copy_offset],
+                result->data, result->size);
+        } else {
+            erts_copy_bits(result->data, bit_offset, 1,
+                (byte*)&copy_buffer->data[*copy_offset], 0, 1,
+                result->size * 8);
+        }
+
+        result->data = &copy_buffer->data[*copy_offset];
+        result->ref_bin = copy_buffer->ref_bin;
+
+        *copy_offset += result->size;
+    }
 }
 
 static int fill_iovec_with_slice(ErlNifEnv *env,
                                  iovec_slice_t *slice,
                                  ErlNifIOVec *iovec) {
-    UWord onheap_offset, iovec_idx;
-    ErlNifBinary onheap_data;
+    ErlNifBinary copy_buffer = {0};
+    UWord copy_offset, iovec_idx;
     Eterm sublist_iterator;
 
-    /* Set up a common refc binary for all on-heap binaries. */
-    if (slice->onheap_size > 0) {
-        if (!enif_alloc_binary(slice->onheap_size, &onheap_data)) {
+    /* Set up a common refc binary for all on-heap and unaligned binaries. */
+    if (slice->copied_size > 0) {
+        if (!enif_alloc_binary(slice->copied_size, &copy_buffer)) {
             return 0;
         }
+
+        ASSERT(copy_buffer.ref_bin != NULL);
     }
 
     sublist_iterator = slice->sublist_start;
-    onheap_offset = 0;
+    copy_offset = 0;
     iovec_idx = 0;
 
     while (sublist_iterator != slice->sublist_end) {
@@ -3423,20 +3491,7 @@ static int fill_iovec_with_slice(ErlNifEnv *env,
         Eterm *cell;
 
         cell = list_val(sublist_iterator);
-        inspect_raw_binary_data(CAR(cell), &raw_data);
-
-        /* If this isn't a refc binary, copy its contents to the onheap buffer
-         * and reference that instead. */
-        if (raw_data.size > 0 && raw_data.ref_bin == NULL) {
-            ASSERT(onheap_offset < onheap_data.size);
-            ASSERT(slice->onheap_size > 0);
-
-            sys_memcpy(&onheap_data.data[onheap_offset],
-                       raw_data.data, raw_data.size);
-
-            raw_data.data = &onheap_data.data[onheap_offset];
-            raw_data.ref_bin = onheap_data.ref_bin;
-        }
+        marshal_iovec_binary(CAR(cell), &copy_buffer, &copy_offset, &raw_data);
 
         while (raw_data.size > 0) {
             UWord chunk_len = MIN(raw_data.size, MAX_SYSIOVEC_IOVLEN);
@@ -3467,16 +3522,18 @@ static int fill_iovec_with_slice(ErlNifEnv *env,
             erts_refc_inc(&refc_binary->intern.refc, 1);
         }
 
-        if (slice->onheap_size > 0) {
+        if (slice->copied_size > 0) {
             /* Transfer ownership to the iovec; we've taken references to it in
              * the above loop. */
-            enif_release_binary(&onheap_data);
+            enif_release_binary(&copy_buffer);
         }
     } else {
-        if (slice->onheap_size > 0) {
-            /* Attach the binary to our environment and let the GC take care of
-             * it after returning. */
-            enif_make_binary(env, &onheap_data);
+        if (slice->copied_size > 0) {
+            /* Attach the binary to our environment and let the next minor GC
+             * get rid of it. This is slightly faster than using the tmp object
+             * list since it avoids off-heap allocations. */
+            erts_build_proc_bin(&MSO(env->proc),
+                alloc_heap(env, PROC_BIN_SIZE), copy_buffer.ref_bin);
         }
     }
 
@@ -3502,19 +3559,14 @@ static int create_iovec_from_slice(ErlNifEnv *env,
         alloc_size = binv_offset;
         alloc_size += slice->iovec_len * sizeof(Binary*);
 
-        /* If we have an environment we'll attach the allocated data to it. The
-         * GC will take care of releasing it later on. */
+        /* When the user passes an environment, we attach the iovec to it so
+         * the user won't have to bother managing it (similar to
+         * enif_inspect_binary). It'll disappear once the environment is
+         * cleaned up. */
         if (env != NULL) {
-            ErlNifBinary gc_bin;
-
-            if (!enif_alloc_binary(alloc_size, &gc_bin)) {
-                return 0;
-            }
-
-            alloc_base = (char*)gc_bin.data;
-            enif_make_binary(env, &gc_bin);
+            alloc_base = alloc_tmp_obj(env, alloc_size, &tmp_alloc_dtor);
         } else {
-            alloc_base = enif_alloc(alloc_size);
+            alloc_base = erts_alloc(ERTS_ALC_T_NIF, alloc_size);
         }
 
         iovec = (ErlNifIOVec*)alloc_base;
@@ -3523,12 +3575,12 @@ static int create_iovec_from_slice(ErlNifEnv *env,
         iovec->flags = 0;
     }
 
-    iovec->size = slice->offheap_size + slice->onheap_size;
+    iovec->size = slice->referenced_size + slice->copied_size;
     iovec->iovcnt = slice->iovec_len;
 
     if(!fill_iovec_with_slice(env, slice, iovec)) {
         if (env == NULL && !(iovec->flags & ERL_NIF_IOVEC_FLAGS_PREALLOC)) {
-            enif_free(iovec);
+            erts_free(ERTS_ALC_T_NIF, iovec);
         }
 
         return 0;
@@ -4218,34 +4270,31 @@ static unsigned calc_checksum(unsigned char* ptr, unsigned size);
 
 struct readonly_check_t
 {
-    struct enif_tmp_obj_t hdr;
     unsigned char* ptr;
     unsigned size;
     unsigned checksum;
 };
 static void add_readonly_check(ErlNifEnv* env, unsigned char* ptr, unsigned sz)
 {
-    ErtsAlcType_t allocator = is_proc_bound(env) ? ERTS_ALC_T_TMP : ERTS_ALC_T_NIF;
-    struct readonly_check_t* obj = erts_alloc(allocator, 
-					      sizeof(struct readonly_check_t));
-    obj->hdr.allocator = allocator;
-    obj->hdr.next = env->tmp_obj_list;
-    env->tmp_obj_list = &obj->hdr;
-    obj->hdr.dtor = &readonly_check_dtor;
+    struct readonly_check_t* obj;
+
+    obj = alloc_tmp_obj(env, sizeof(struct readonly_check_t),
+        &readonly_check_dtor);
+
     obj->ptr = ptr;
     obj->size = sz;
-    obj->checksum = calc_checksum(ptr, sz);    
+    obj->checksum = calc_checksum(ptr, sz);
 }
-static void readonly_check_dtor(struct enif_tmp_obj_t* o)
+static void readonly_check_dtor(struct enif_tmp_obj_t* tmp_obj)
 {
-    struct readonly_check_t* obj = (struct readonly_check_t*) o;
-    unsigned chksum = calc_checksum(obj->ptr, obj->size);
-    if (chksum != obj->checksum) { 
+    struct readonly_check_t* ro_check = (struct readonly_check_t*)&tmp_obj[1];
+    unsigned chksum = calc_checksum(ro_check->ptr, ro_check->size);
+    if (chksum != ro_check->checksum) { 
 	fprintf(stderr, "\r\nReadonly data written by NIF, checksums differ"
-		" %x != %x\r\nABORTING\r\n", chksum, obj->checksum);
+		" %x != %x\r\nABORTING\r\n", chksum, ro_check->checksum);
 	abort();
     }
-    erts_free(obj->hdr.allocator,  obj);
+    erts_free(tmp_obj->allocator, tmp_obj);
 }
 static unsigned calc_checksum(unsigned char* ptr, unsigned size)
 {