[PATCH 0/3] Minor compression heuristic cleanups

[PATCH 0/3] Minor compression heuristic cleanups

[David Sterba]

The callback pointers for radix_sort are not needed, we don't plan to
export the function now. The compiler is smart enough to replace the
indirect calls with direct ones, so there's no change in the resulting
asm code.

David Sterba (3):
  btrfs: heuristic: open code get_num callback of radix sort
  btrfs: heuristic: open code copy_call callback of radix sort
  btrfs: heuristic: call get4bits directly

 fs/btrfs/compression.c | 42 +++++++++++-------------------------------
 1 file changed, 11 insertions(+), 31 deletions(-)

-- 
2.15.1

[PATCH 1/3] btrfs: heuristic: open code get_num callback of radix sort

[David Sterba]

The callback is trivial and we don't need the abstraction for our
purposes. Let's open code it and also make the array types explicit.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 28c3940062b7..37a69d4b04ce 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -1318,12 +1318,6 @@ static void copy_cell(void *dst, int dest_i, void *src, int src_i)
 	dstv[dest_i] = srcv[src_i];
 }
 
-static u64 get_num(const void *a, int i)
-{
-	struct bucket_item *av = (struct bucket_item *)a;
-	return av[i].count;
-}
-
 /*
  * Use 4 bits as radix base
  * Use 16 u32 counters for calculating new possition in buf array
@@ -1332,12 +1326,11 @@ static u64 get_num(const void *a, int i)
  * @array_buf - buffer array to store sorting results
  *              must be equal in size to @array
  * @num       - array size
- * @get_num   - function to extract number from array
  * @copy_cell - function to copy data from array to array_buf and vice versa
  * @get4bits  - function to get 4 bits from number at specified offset
  */
-static void radix_sort(void *array, void *array_buf, int num,
-		       u64 (*get_num)(const void *, int i),
+static void radix_sort(struct bucket_item *array, struct bucket_item *array_buf,
+		       int num,
 		       void (*copy_cell)(void *dest, int dest_i,
 					 void* src, int src_i),
 		       u8 (*get4bits)(u64 num, int shift))
@@ -1355,9 +1348,9 @@ static void radix_sort(void *array, void *array_buf, int num,
 	 * Try avoid useless loop iterations for small numbers stored in big
 	 * counters.  Example: 48 33 4 ... in 64bit array
 	 */
-	max_num = get_num(array, 0);
+	max_num = array[0].count;
 	for (i = 1; i < num; i++) {
-		buf_num = get_num(array, i);
+		buf_num = array[i].count;
 		if (buf_num > max_num)
 			max_num = buf_num;
 	}
@@ -1370,7 +1363,7 @@ static void radix_sort(void *array, void *array_buf, int num,
 		memset(counters, 0, sizeof(counters));
 
 		for (i = 0; i < num; i++) {
-			buf_num = get_num(array, i);
+			buf_num = array[i].count;
 			addr = get4bits(buf_num, shift);
 			counters[addr]++;
 		}
@@ -1379,7 +1372,7 @@ static void radix_sort(void *array, void *array_buf, int num,
 			counters[i] += counters[i - 1];
 
 		for (i = num - 1; i >= 0; i--) {
-			buf_num = get_num(array, i);
+			buf_num = array[i].count;
 			addr = get4bits(buf_num, shift);
 			counters[addr]--;
 			new_addr = counters[addr];
@@ -1397,7 +1390,7 @@ static void radix_sort(void *array, void *array_buf, int num,
 		memset(counters, 0, sizeof(counters));
 
 		for (i = 0; i < num; i ++) {
-			buf_num = get_num(array_buf, i);
+			buf_num = array_buf[i].count;
 			addr = get4bits(buf_num, shift);
 			counters[addr]++;
 		}
@@ -1406,7 +1399,7 @@ static void radix_sort(void *array, void *array_buf, int num,
 			counters[i] += counters[i - 1];
 
 		for (i = num - 1; i >= 0; i--) {
-			buf_num = get_num(array_buf, i);
+			buf_num = array_buf[i].count;
 			addr = get4bits(buf_num, shift);
 			counters[addr]--;
 			new_addr = counters[addr];
@@ -1444,7 +1437,7 @@ static int byte_core_set_size(struct heuristic_ws *ws)
 	struct bucket_item *bucket = ws->bucket;
 
 	/* Sort in reverse order */
-	radix_sort(ws->bucket, ws->bucket_b, BUCKET_SIZE, get_num, copy_cell,
+	radix_sort(ws->bucket, ws->bucket_b, BUCKET_SIZE, copy_cell,
 			get4bits);
 
 	for (i = 0; i < BYTE_CORE_SET_LOW; i++)
-- 
2.15.1

[PATCH 2/3] btrfs: heuristic: open code copy_call callback of radix sort

[David Sterba]

The callback is trivial and we don't need the abstraction for our
purposes. Let's open code it.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 37a69d4b04ce..935acabc0ea7 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -1311,13 +1311,6 @@ static u8 get4bits(u64 num, int shift) {
 	return low4bits;
 }
 
-static void copy_cell(void *dst, int dest_i, void *src, int src_i)
-{
-	struct bucket_item *dstv = (struct bucket_item *)dst;
-	struct bucket_item *srcv = (struct bucket_item *)src;
-	dstv[dest_i] = srcv[src_i];
-}
-
 /*
  * Use 4 bits as radix base
  * Use 16 u32 counters for calculating new possition in buf array
@@ -1326,13 +1319,10 @@ static void copy_cell(void *dst, int dest_i, void *src, int src_i)
  * @array_buf - buffer array to store sorting results
  *              must be equal in size to @array
  * @num       - array size
- * @copy_cell - function to copy data from array to array_buf and vice versa
  * @get4bits  - function to get 4 bits from number at specified offset
  */
 static void radix_sort(struct bucket_item *array, struct bucket_item *array_buf,
 		       int num,
-		       void (*copy_cell)(void *dest, int dest_i,
-					 void* src, int src_i),
 		       u8 (*get4bits)(u64 num, int shift))
 {
 	u64 max_num;
@@ -1376,7 +1366,7 @@ static void radix_sort(struct bucket_item *array, struct bucket_item *array_buf,
 			addr = get4bits(buf_num, shift);
 			counters[addr]--;
 			new_addr = counters[addr];
-			copy_cell(array_buf, new_addr, array, i);
+			array_buf[new_addr] = array[i];
 		}
 
 		shift += RADIX_BASE;
@@ -1403,7 +1393,7 @@ static void radix_sort(struct bucket_item *array, struct bucket_item *array_buf,
 			addr = get4bits(buf_num, shift);
 			counters[addr]--;
 			new_addr = counters[addr];
-			copy_cell(array, new_addr, array_buf, i);
+			array[new_addr] = array_buf[i];
 		}
 
 		shift += RADIX_BASE;
@@ -1437,8 +1427,7 @@ static int byte_core_set_size(struct heuristic_ws *ws)
 	struct bucket_item *bucket = ws->bucket;
 
 	/* Sort in reverse order */
-	radix_sort(ws->bucket, ws->bucket_b, BUCKET_SIZE, copy_cell,
-			get4bits);
+	radix_sort(ws->bucket, ws->bucket_b, BUCKET_SIZE, get4bits);
 
 	for (i = 0; i < BYTE_CORE_SET_LOW; i++)
 		coreset_sum += bucket[i].count;
-- 
2.15.1

[PATCH 3/3] btrfs: heuristic: call get4bits directly

[David Sterba]

As it's a single instance and local to the file, we don't need to pass
it as an argument.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 935acabc0ea7..208334aa6c6e 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -1319,11 +1319,9 @@ static u8 get4bits(u64 num, int shift) {
  * @array_buf - buffer array to store sorting results
  *              must be equal in size to @array
  * @num       - array size
- * @get4bits  - function to get 4 bits from number at specified offset
  */
 static void radix_sort(struct bucket_item *array, struct bucket_item *array_buf,
-		       int num,
-		       u8 (*get4bits)(u64 num, int shift))
+		       int num)
 {
 	u64 max_num;
 	u64 buf_num;
@@ -1427,7 +1425,7 @@ static int byte_core_set_size(struct heuristic_ws *ws)
 	struct bucket_item *bucket = ws->bucket;
 
 	/* Sort in reverse order */
-	radix_sort(ws->bucket, ws->bucket_b, BUCKET_SIZE, get4bits);
+	radix_sort(ws->bucket, ws->bucket_b, BUCKET_SIZE);
 
 	for (i = 0; i < BYTE_CORE_SET_LOW; i++)
 		coreset_sum += bucket[i].count;
-- 
2.15.1

Re: [PATCH 0/3] Minor compression heuristic cleanups

[Timofey Titovets]

2017-12-12 23:55 GMT+03:00 David Sterba <dsterba@suse.com>:
> The callback pointers for radix_sort are not needed, we don't plan to
> export the function now. The compiler is smart enough to replace the
> indirect calls with direct ones, so there's no change in the resulting
> asm code.
>
> David Sterba (3):
>   btrfs: heuristic: open code get_num callback of radix sort
>   btrfs: heuristic: open code copy_call callback of radix sort
>   btrfs: heuristic: call get4bits directly
>
>  fs/btrfs/compression.c | 42 +++++++++++-------------------------------
>  1 file changed, 11 insertions(+), 31 deletions(-)
>
> --
> 2.15.1
>

Thanks!
On whole series:
Reviewed-by: Timofey Titovets

-- 
Have a nice day,
Timofey.