Tarantool development patches archive
 help / color / mirror / Atom feed
* [PATCH] vinyl: do not account bloom filters to runtime quota
@ 2018-10-26 10:18 Vladimir Davydov
  2018-10-26 10:49 ` Vladimir Davydov
  0 siblings, 1 reply; 2+ messages in thread
From: Vladimir Davydov @ 2018-10-26 10:18 UTC (permalink / raw)
  To: kostja; +Cc: tarantool-patches

Back when bloom filters were introduced, neither box.info.memory() nor
box.stat.vinyl().memory didn't exist so bloom filters were accounted to
box.runtime.info().used for lack of a better place. Now, there's no
point to account them there. In fact, it's confusing, because bloom
filters are allocated with malloc(), not from the runtime arena, so
let's drop it.
---
https://github.com/tarantool/tarantool/commits/dv/vy-dont-account-bloom-to-runtime-arena

 src/box/tuple_bloom.c  | 10 ++++------
 src/lib/salad/bloom.c  | 22 +++++-----------------
 src/lib/salad/bloom.h  | 10 +++-------
 test/unit/bloom.cc     | 18 ++++++------------
 test/unit/bloom.result |  4 ----
 5 files changed, 18 insertions(+), 46 deletions(-)

diff --git a/src/box/tuple_bloom.c b/src/box/tuple_bloom.c
index dc406985..0ea552aa 100644
--- a/src/box/tuple_bloom.c
+++ b/src/box/tuple_bloom.c
@@ -39,7 +39,6 @@
 #include <msgpuck.h>
 #include "diag.h"
 #include "errcode.h"
-#include "memory.h"
 #include "key_def.h"
 #include "tuple.h"
 #include "tuple_hash.h"
@@ -143,8 +142,7 @@ tuple_bloom_new(struct tuple_bloom_builder *builder, double fpr)
 		for (uint32_t j = 0; j < i; j++)
 			part_fpr /= bloom_fpr(&bloom->parts[j], count);
 		part_fpr = MIN(part_fpr, 0.5);
-		if (bloom_create(&bloom->parts[i], count,
-				 part_fpr, runtime.quota) != 0) {
+		if (bloom_create(&bloom->parts[i], count, part_fpr) != 0) {
 			diag_set(OutOfMemory, 0, "bloom_create",
 				 "tuple bloom part");
 			tuple_bloom_delete(bloom);
@@ -161,7 +159,7 @@ void
 tuple_bloom_delete(struct tuple_bloom *bloom)
 {
 	for (uint32_t i = 0; i < bloom->part_count; i++)
-		bloom_destroy(&bloom->parts[i], runtime.quota);
+		bloom_destroy(&bloom->parts[i]);
 	free(bloom);
 }
 
@@ -251,7 +249,7 @@ tuple_bloom_decode_part(struct bloom *part, const char **data)
 	part->hash_count = mp_decode_uint(data);
 	size_t store_size = mp_decode_binl(data);
 	assert(store_size == bloom_store_size(part));
-	if (bloom_load_table(part, *data, runtime.quota) != 0) {
+	if (bloom_load_table(part, *data) != 0) {
 		diag_set(OutOfMemory, store_size, "bloom_load_table",
 			 "tuple bloom part");
 		return -1;
@@ -329,7 +327,7 @@ tuple_bloom_decode_legacy(const char **data)
 
 	size_t store_size = mp_decode_binl(data);
 	assert(store_size == bloom_store_size(&bloom->parts[0]));
-	if (bloom_load_table(&bloom->parts[0], *data, runtime.quota) != 0) {
+	if (bloom_load_table(&bloom->parts[0], *data) != 0) {
 		diag_set(OutOfMemory, store_size, "bloom_load_table",
 			 "tuple bloom part");
 		free(bloom);
diff --git a/src/lib/salad/bloom.c b/src/lib/salad/bloom.c
index d68692ba..3460465a 100644
--- a/src/lib/salad/bloom.c
+++ b/src/lib/salad/bloom.c
@@ -41,7 +41,7 @@
 
 int
 bloom_create(struct bloom *bloom, uint32_t number_of_values,
-	     double false_positive_rate, struct quota *quota)
+	     double false_positive_rate)
 {
 	/* Optimal hash_count and bit count calculation */
 	uint16_t hash_count = ceil(log(false_positive_rate) / log(0.5));
@@ -49,14 +49,9 @@ bloom_create(struct bloom *bloom, uint32_t number_of_values,
 	uint32_t block_bits = CHAR_BIT * sizeof(struct bloom_block);
 	uint32_t block_count = (bit_count + block_bits - 1) / block_bits;
 
-	if (quota_use(quota, block_count * sizeof(*bloom->table)) < 0)
-		return -1;
-
 	bloom->table = calloc(block_count, sizeof(*bloom->table));
-	if (bloom->table == NULL) {
-		quota_release(quota, block_count * sizeof(*bloom->table));
+	if (bloom->table == NULL)
 		return -1;
-	}
 
 	bloom->table_size = block_count;
 	bloom->hash_count = hash_count;
@@ -64,9 +59,8 @@ bloom_create(struct bloom *bloom, uint32_t number_of_values,
 }
 
 void
-bloom_destroy(struct bloom *bloom, struct quota *quota)
+bloom_destroy(struct bloom *bloom)
 {
-	quota_release(quota, bloom->table_size * sizeof(*bloom->table));
 	free(bloom->table);
 }
 
@@ -98,18 +92,12 @@ bloom_store(const struct bloom *bloom, char *table)
 }
 
 int
-bloom_load_table(struct bloom *bloom, const char *table, struct quota *quota)
+bloom_load_table(struct bloom *bloom, const char *table)
 {
 	size_t size = bloom->table_size * sizeof(struct bloom_block);
-	if (quota_use(quota, size) < 0) {
-		bloom->table = NULL;
-		return -1;
-	}
 	bloom->table = malloc(size);
-	if (bloom->table == NULL) {
-		quota_release(quota, size);
+	if (bloom->table == NULL)
 		return -1;
-	}
 	memcpy(bloom->table, table, size);
 	return 0;
 }
diff --git a/src/lib/salad/bloom.h b/src/lib/salad/bloom.h
index 32deb81a..c5f9e029 100644
--- a/src/lib/salad/bloom.h
+++ b/src/lib/salad/bloom.h
@@ -48,7 +48,6 @@
 #include <stddef.h>
 #include <limits.h>
 #include "bit/bit.h"
-#include "small/quota.h"
 
 #if defined(__cplusplus)
 extern "C" {
@@ -88,21 +87,19 @@ struct bloom {
  * @param bloom - structure to initialize
  * @param number_of_values - estimated number of values to be added
  * @param false_positive_rate - desired false positive rate
- * @param quota - quota for memory allocation
  * @return 0 - OK, -1 - memory error
  */
 int
 bloom_create(struct bloom *bloom, uint32_t number_of_values,
-	     double false_positive_rate, struct quota *quota);
+	     double false_positive_rate);
 
 /**
  * Free resources of the bloom filter
  *
  * @param bloom - the bloom filter
- * @param quota - quota for memory deallocation
  */
 void
-bloom_destroy(struct bloom *bloom, struct quota *quota);
+bloom_destroy(struct bloom *bloom);
 
 /**
  * Add a value into the data set
@@ -156,11 +153,10 @@ bloom_store(const struct bloom *bloom, char *table);
  *
  * @param bloom - structure to load to
  * @param table - data to load
- * @param quota - quota for memory allocation
  * @return 0 - OK, -1 - memory error
  */
 int
-bloom_load_table(struct bloom *bloom, const char *table, struct quota *quota);
+bloom_load_table(struct bloom *bloom, const char *table);
 
 /* }}} API declaration */
 
diff --git a/test/unit/bloom.cc b/test/unit/bloom.cc
index b8a2ef13..2e5b9941 100644
--- a/test/unit/bloom.cc
+++ b/test/unit/bloom.cc
@@ -14,8 +14,6 @@ void
 simple_test()
 {
 	cout << "*** " << __func__ << " ***" << endl;
-	struct quota q;
-	quota_init(&q, 100500);
 	srand(time(0));
 	uint32_t error_count = 0;
 	uint32_t fp_rate_too_big = 0;
@@ -24,7 +22,7 @@ simple_test()
 		uint64_t false_positive = 0;
 		for (uint32_t count = 1000; count <= 10000; count *= 2) {
 			struct bloom bloom;
-			bloom_create(&bloom, count, p, &q);
+			bloom_create(&bloom, count, p);
 			unordered_set<uint32_t> check;
 			for (uint32_t i = 0; i < count; i++) {
 				uint32_t val = rand() % (count * 10);
@@ -41,7 +39,7 @@ simple_test()
 				if (!has && bloom_possible)
 					false_positive++;
 			}
-			bloom_destroy(&bloom, &q);
+			bloom_destroy(&bloom);
 		}
 		double fp_rate = (double)false_positive / tests;
 		if (fp_rate > p + 0.001)
@@ -49,15 +47,12 @@ simple_test()
 	}
 	cout << "error_count = " << error_count << endl;
 	cout << "fp_rate_too_big = " << fp_rate_too_big << endl;
-	cout << "memory after destruction = " << quota_used(&q) << endl << endl;
 }
 
 void
 store_load_test()
 {
 	cout << "*** " << __func__ << " ***" << endl;
-	struct quota q;
-	quota_init(&q, 100500);
 	srand(time(0));
 	uint32_t error_count = 0;
 	uint32_t fp_rate_too_big = 0;
@@ -66,7 +61,7 @@ store_load_test()
 		uint64_t false_positive = 0;
 		for (uint32_t count = 300; count <= 3000; count *= 10) {
 			struct bloom bloom;
-			bloom_create(&bloom, count, p, &q);
+			bloom_create(&bloom, count, p);
 			unordered_set<uint32_t> check;
 			for (uint32_t i = 0; i < count; i++) {
 				uint32_t val = rand() % (count * 10);
@@ -76,9 +71,9 @@ store_load_test()
 			struct bloom test = bloom;
 			char *buf = (char *)malloc(bloom_store_size(&bloom));
 			bloom_store(&bloom, buf);
-			bloom_destroy(&bloom, &q);
+			bloom_destroy(&bloom);
 			memset(&bloom, '#', sizeof(bloom));
-			bloom_load_table(&test, buf, &q);
+			bloom_load_table(&test, buf);
 			free(buf);
 			for (uint32_t i = 0; i < count * 10; i++) {
 				bool has = check.find(i) != check.end();
@@ -90,7 +85,7 @@ store_load_test()
 				if (!has && bloom_possible)
 					false_positive++;
 			}
-			bloom_destroy(&test, &q);
+			bloom_destroy(&test);
 		}
 		double fp_rate = (double)false_positive / tests;
 		double excess = fp_rate / p;
@@ -99,7 +94,6 @@ store_load_test()
 	}
 	cout << "error_count = " << error_count << endl;
 	cout << "fp_rate_too_big = " << fp_rate_too_big << endl;
-	cout << "memory after destruction = " << quota_used(&q) << endl << endl;
 }
 
 int
diff --git a/test/unit/bloom.result b/test/unit/bloom.result
index 1b1b1f94..91193a61 100644
--- a/test/unit/bloom.result
+++ b/test/unit/bloom.result
@@ -1,10 +1,6 @@
 *** simple_test ***
 error_count = 0
 fp_rate_too_big = 0
-memory after destruction = 0
-
 *** store_load_test ***
 error_count = 0
 fp_rate_too_big = 0
-memory after destruction = 0
-
-- 
2.11.0

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] vinyl: do not account bloom filters to runtime quota
  2018-10-26 10:18 [PATCH] vinyl: do not account bloom filters to runtime quota Vladimir Davydov
@ 2018-10-26 10:49 ` Vladimir Davydov
  0 siblings, 0 replies; 2+ messages in thread
From: Vladimir Davydov @ 2018-10-26 10:49 UTC (permalink / raw)
  To: kostja; +Cc: tarantool-patches

Pushed to 1.10 as trivial.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-10-26 10:49 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-26 10:18 [PATCH] vinyl: do not account bloom filters to runtime quota Vladimir Davydov
2018-10-26 10:49 ` Vladimir Davydov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox