threads / patch / 58433

v2reftable: use a pointer for pq_entry param

Subject: [PATCH v2] reftable: use a pointer for pq_entry param

## tl;dr

4 messages between Sep 15, 2022 and Sep 15, 2022. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Elijah Conners· Sep 15, 2022, 03:37 UTC · lore

The speed of the merged_iter_pqueue_add() can be improved by using a pointer to the pq_entry struct, which is 96 bytes. Since the pq_entry param is worked directly on the stack and does not currently have a pointer to it, the merged_iter_pqueue_add() function is slightly slower.

References to pq_entry in reftable have typically included pointers, such as both of the params for pq_less().

Since we are working with pointers in the pq_entry param, as keenly pointed out, the pq_entry param has also been made into a const since the contents of the pq_entry param are copied and not manipulated.

Signed-off-by: Elijah Conners <business@elijahpepe.com>
---
 reftable/merged.c  | 4 ++--
 reftable/pq.c      | 4 ++--
 reftable/pq.h      | 2 +-
 reftable/pq_test.c | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)
Show changes to 4 files +6 −7

reftable/merged.c, reftable/pq.c, reftable/pq.h, reftable/pq_test.c

diff --git a/reftable/merged.c b/reftable/merged.c
index 2a6efa110d..5ded470c08 100644
--- a/reftable/merged.c
+++ b/reftable/merged.c
@@ -36,7 +36,7 @@ static int merged_iter_init(struct merged_iter *mi)
                                .rec = rec,
                                .index = i,
                        };
-                       merged_iter_pqueue_add(&mi->pq, e);
+                       merged_iter_pqueue_add(&mi->pq, &e);
                }
        }

@@ -71,7 +71,7 @@ static int merged_iter_advance_nonnull_subiter(struct merged_iter *mi,
                return 0;
        }

-       merged_iter_pqueue_add(&mi->pq, e);
+       merged_iter_pqueue_add(&mi->pq, &e);
        return 0;
 }

diff --git a/reftable/pq.c b/reftable/pq.c
index 96ca6dd37b..dcefeb793a 100644
--- a/reftable/pq.c
+++ b/reftable/pq.c
@@ -71,7 +71,7 @@ struct pq_entry merged_iter_pqueue_remove(struct merged_iter_pqueue *pq)
        return e;
 }

-void merged_iter_pqueue_add(struct merged_iter_pqueue *pq, struct pq_entry e)
+void merged_iter_pqueue_add(struct merged_iter_pqueue *pq, const struct pq_entry *e)
 {
        int i = 0;

@@ -81,7 +81,7 @@ void merged_iter_pqueue_add(struct merged_iter_pqueue *pq, struct pq_entry e)
                                            pq->cap * sizeof(struct pq_entry));
        }

-       pq->heap[pq->len++] = e;
+       pq->heap[pq->len++] = *e;
        i = pq->len - 1;
        while (i > 0) {
                int j = (i - 1) / 2;
diff --git a/reftable/pq.h b/reftable/pq.h
index 56fc1b6d87..e85bac9b52 100644
--- a/reftable/pq.h
+++ b/reftable/pq.h
@@ -26,7 +26,7 @@ struct pq_entry merged_iter_pqueue_top(struct merged_iter_pqueue pq);
 int merged_iter_pqueue_is_empty(struct merged_iter_pqueue pq);
 void merged_iter_pqueue_check(struct merged_iter_pqueue pq);
 struct pq_entry merged_iter_pqueue_remove(struct merged_iter_pqueue *pq);
-void merged_iter_pqueue_add(struct merged_iter_pqueue *pq, struct pq_entry e);
+void merged_iter_pqueue_add(struct merged_iter_pqueue *pq, const struct pq_entry *e);
 void merged_iter_pqueue_release(struct merged_iter_pqueue *pq);
 int pq_less(struct pq_entry *a, struct pq_entry *b);

diff --git a/reftable/pq_test.c b/reftable/pq_test.c
index 7de5e886f3..011b5c7502 100644
--- a/reftable/pq_test.c
+++ b/reftable/pq_test.c
@@ -46,7 +46,7 @@ static void test_pq(void)
                                               .u.ref = {
                                                       .refname = names[i],
                                               } } };
-               merged_iter_pqueue_add(&pq, e);
+               merged_iter_pqueue_add(&pq, &e);
                merged_iter_pqueue_check(pq);
                i = (i * 7) % N;
        } while (i != 1);
--
2.29.2.windows.2
Junio C Hamano· Sep 15, 2022, 18:01 UTC · re: Elijah Conners · lore

Re: [PATCH v2] reftable: use a pointer for pq_entry param

Hmph. I do not know what went wrong. Somebody between your "git format-patch" and the mailing list archive ate tabs and spitted out spaces, it seems. I'll try to fix it up.

Thanks.
$ git am -s ./+ec
.git/rebase-apply/patch:13: indent with spaces.
                               .rec = rec,
.git/rebase-apply/patch:14: indent with spaces.
                               .index = i,
.git/rebase-apply/patch:15: indent with spaces.
                       };
.git/rebase-apply/patch:17: indent with spaces.
                       merged_iter_pqueue_add(&mi->pq, &e);
.git/rebase-apply/patch:18: indent with spaces.
               }
error: patch failed: reftable/merged.c:36
error: reftable/merged.c: patch does not apply
error: patch failed: reftable/pq.c:71
error: reftable/pq.c: patch does not apply
error: patch failed: reftable/pq_test.c:46
error: reftable/pq_test.c: patch does not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Applying: reftable: use a pointer for pq_entry param
Patch failed at 0001 reftable: use a pointer for pq_entry param
Elijah Conners· Sep 15, 2022, 20:19 UTC · re: Junio C Hamano · lore

Re: [PATCH v2] reftable: use a pointer for pq_entry param

Junio C Hamano <gitster@pobox.com> writes:
 > Hmph.  I do not know what went wrong.  Somebody between your "git
 > format-patch" and the mailing list archive ate tabs and spitted out
 > spaces, it seems.  I'll try to fix it up.
Looks like my mail server tried to convert the spaces to tabs. If you could fix it, that would be great, otherwise I'll submit another patch in four hours.
Junio C Hamano· Sep 15, 2022, 20:42 UTC · re: Elijah Conners · lore

Re: [PATCH v2] reftable: use a pointer for pq_entry param

Elijah Conners <business@elijahpepe.com> writes:
Show 8 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>  > Hmph.  I do not know what went wrong.  Somebody between your "git
>  > format-patch" and the mailing list archive ate tabs and spitted out
>  > spaces, it seems.  I'll try to fix it up.
>
> Looks like my mail server tried to convert the spaces to tabs. If
> you could fix it, that would be great, otherwise I'll submit
> another patch in four hours.

I just pushed out the first integration result of today, which contains this topic.

Thanks.

← back to recent threads