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

Re: [PATCH 7/7] reftable/reader: add comments to `table_iter_next()`

From
John Cai <johncai86@gmail.com>
Date
Feb 9, 2024, 16:01 UTC
Message-ID
<5335108F-4A21-4429-9BDF-3923D96884C0@gmail.com>
In-Reply-To
<2f1f1dd95e1cc360bde3547bd18e227a9c326e13.1706782841.git.ps@pks.im>
Hi Patrick,
On 1 Feb 2024, at 5:25, Patrick Steinhardt wrote:
Show 53 quoted lines
> While working on the optimizations in the preceding patches I stumbled
> upon `table_iter_next()` multiple times. It is quite easy to miss the
> fact that we don't call `table_iter_next_in_block()` twice, but that the
> second call is in fact `table_iter_next_block()`.
>
> Add comments to explain what exactly is going on here to make things
> more obvious. While at it, touch up the code to conform to our code
> style better.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  reftable/reader.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/reftable/reader.c b/reftable/reader.c
> index 64dc366fb1..add7d57f0b 100644
> --- a/reftable/reader.c
> +++ b/reftable/reader.c
> @@ -357,24 +357,32 @@ static int table_iter_next(struct table_iter *ti, struct reftable_record *rec)
>
>  	while (1) {
>  		struct table_iter next = TABLE_ITER_INIT;
> -		int err = 0;
> -		if (ti->is_finished) {
> +		int err;
> +
> +		if (ti->is_finished)
>  			return 1;
> -		}
>
> +		/*
> +		 * Check whether the current block still has more records. If
> +		 * so, return it. If the iterator returns positive then the
> +		 * current block has been exhausted.
> +		 */
>  		err = table_iter_next_in_block(ti, rec);
> -		if (err <= 0) {
> +		if (err <= 0)
>  			return err;
> -		}
>
> +		/*
> +		 * Otherwise, we need to continue to the next block in the
> +		 * table and retry. If there are no more blocks then the
> +		 * iterator is drained.
> +		 */
>  		err = table_iter_next_block(&next, ti);
> -		if (err != 0) {
> -			ti->is_finished = 1;
> -		}
>  		table_iter_block_done(ti);
> -		if (err != 0) {
> +		if (err) {

what's the reason for moving the if statement that handles err down after table_iter_block_done?

Show 9 quoted lines
> +			ti->is_finished = 1;
>  			return err;
>  		}
> +
>  		table_iter_copy_from(ti, &next);
>  		block_iter_close(&next.bi);
>  	}
> -- 
> 2.43.GIT

Thanks John

Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 12 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.