git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 0/7] reftable: improve ref iteration performance

From
Patrick Steinhardt <ps@pks.im>
Date
Feb 12, 2024, 08:32 UTC
Message-ID
<cover.1707726654.git.ps@pks.im>
In-Reply-To
<cover.1706782841.git.ps@pks.im>
Hi,

this is the second version of my patch series that improves ref iteration performance. There are no code changes compared to v1, but a few improvements to the commit messages.

Thanks!
Patrick
Patrick Steinhardt (7):
  reftable/record: introduce function to compare records by key
  reftable/merged: allocation-less dropping of shadowed records
  reftable/merged: skip comparison for records of the same subiter
  reftable/pq: allocation-less comparison of entry keys
  reftable/block: swap buffers instead of copying
  reftable/record: don't try to reallocate ref record name
  reftable/reader: add comments to `table_iter_next()`
 reftable/block.c  |  3 +--
 reftable/merged.c | 19 +++++++-------
 reftable/merged.h |  2 --
 reftable/pq.c     | 13 +--------
 reftable/reader.c | 26 +++++++++++-------
 reftable/record.c | 67 ++++++++++++++++++++++++++++++++++++++++++++---
 reftable/record.h |  7 +++++
 7 files changed, 100 insertions(+), 37 deletions(-)
Range-diff against v1:
1:  fadabec696 ! 1:  bcdb5a2bf0 reftable/record: introduce function to compare records by key
    @@ Commit message
         In some places we need to sort reftable records by their keys to
         determine their ordering. This is done by first formatting the keys into
         a `struct strbuf` and then using `strbuf_cmp()` to compare them. This
    -    logic is needlessly roundabout and can end up costing quite a bit fo CPU
    +    logic is needlessly roundabout and can end up costing quite a bit of CPU
         cycles, both due to the allocation and formatting logic.
     
    -    Introduce a new `reftable_record_cmp()` function that knows to compare
    -    two records with each other without requiring allocations.
    +    Introduce a new `reftable_record_cmp()` function that knows how to
    +    compare two records with each other without requiring allocations.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
2:  576a96f2e5 = 2:  2364a0fa33 reftable/merged: allocation-less dropping of shadowed records
3:  0ca86eba71 ! 3:  205e6b488b reftable/merged: skip comparison for records of the same subiter
    @@ Commit message
         to return.
     
         There is an edge case here where we can skip that comparison: when the
    -    record in the priority queue comes from the same subiterator than the
    +    record in the priority queue comes from the same subiterator as the
         record we are about to return then we know that its key must be larger
         than the key of the record we are about to return. This property is
         guaranteed by the sub-iterators, and if it didn't hold then the whole
4:  1c9c19a3b3 = 4:  fd09ba70fe reftable/pq: allocation-less comparison of entry keys
5:  ac3d43c001 = 5:  2317aa43b9 reftable/block: swap buffers instead of copying
6:  41dff8731c = 6:  8c67491504 reftable/record: don't try to reallocate ref record name
7:  2f1f1dd95e ! 7:  167f67fad8 reftable/reader: add comments to `table_iter_next()`
    @@ Commit message
         more obvious. While at it, touch up the code to conform to our code
         style better.
     
    +    Note that one of the refactorings merges two conditional blocks into
    +    one. Before, we had the following code:
    +
    +    ```
    +    err = table_iter_next_block(&next, ti
    +    if (err != 0) {
    +            ti->is_finished = 1;
    +    }
    +    table_iter_block_done(ti);
    +    if (err != 0) {
    +            return err;
    +    }
    +    ```
    +
    +    As `table_iter_block_done()` does not care about `is_finished`, the
    +    conditional blocks can be merged into one block:
    +
    +    ```
    +    err = table_iter_next_block(&next, ti
    +    table_iter_block_done(ti);
    +    if (err != 0) {
    +            ti->is_finished = 1;
    +            return err;
    +    }
    +    ```
    +
    +    This is both easier to reason about and more performant because we have
    +    one branch less.
    +
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## reftable/reader.c ##
base-commit: c875e0b8e036c12cfbf6531962108a063c7a821c
-- 
2.43.GIT
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 14 of 23 in “reftable: improve ref iteration performance”
  1. 0/7 reftable: improve ref iteration performancePatrick Steinhardt, Feb 1, 2024
  2. 1/7 reftable/record: introduce function to compare records by keyPatrick Steinhardt, Feb 1, 2024
  3. Eric SunshineFeb 1, 2024
  4. 2/7 reftable/merged: allocation-less dropping of shadowed recordsPatrick Steinhardt, Feb 1, 2024
  5. 3/7 reftable/merged: skip comparison for records of the same subiterPatrick Steinhardt, Feb 1, 2024
  6. Eric SunshineFeb 1, 2024
  7. Patrick SteinhardtFeb 2, 2024
  8. 4/7 reftable/pq: allocation-less comparison of entry keysPatrick Steinhardt, Feb 1, 2024
  9. 5/7 reftable/block: swap buffers instead of copyingPatrick Steinhardt, Feb 1, 2024
  10. 6/7 reftable/record: don't try to reallocate ref record namePatrick Steinhardt, Feb 1, 2024
  11. 7/7 reftable/reader: add comments to `table_iter_next()`Patrick Steinhardt, Feb 1, 2024
  12. John CaiFeb 9, 2024
  13. Patrick SteinhardtFeb 12, 2024
  14. 0/7 reftable: improve ref iteration performancePatrick Steinhardt, Feb 12, 2024
  15. 1/7 reftable/record: introduce function to compare records by keyPatrick Steinhardt, Feb 12, 2024
  16. 2/7 reftable/merged: allocation-less dropping of shadowed recordsPatrick Steinhardt, Feb 12, 2024
  17. 3/7 reftable/merged: skip comparison for records of the same subiterPatrick Steinhardt, Feb 12, 2024
  18. 4/7 reftable/pq: allocation-less comparison of entry keysPatrick Steinhardt, Feb 12, 2024
  19. 5/7 reftable/block: swap buffers instead of copyingPatrick Steinhardt, Feb 12, 2024
  20. 6/7 reftable/record: don't try to reallocate ref record namePatrick Steinhardt, Feb 12, 2024
  21. 7/7 reftable/reader: add comments to `table_iter_next()`Patrick Steinhardt, Feb 12, 2024
  22. Junio C HamanoFeb 12, 2024
  23. Patrick SteinhardtFeb 13, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.