On Wed, Jun 25, 2025 at 2:59 AM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
>
> I liked this version more. I agree that the asserts were causing some confusion.
Thanks for the review!
Sawada-san, do you have any objection to this being merged in
master/18? I know you had said changing the if statements to asserts
did not feel like a bug fix. However, now that the asserts have been
removed, do you still feel this way?
I feel simplifying the counters is a clarity improvement and would
prefer we have it before branching, but I would hold off if you still
felt this way even with the asserts removed.
(I had originally misunderstood and didn't realize that the page bit
must be set if the VM bit is set, so that is why I had a guard in the
empty page case. And for lazy_vacuum_heap_page(), I was being overly
cautious at the expense of clarity.)
> nitpick:
> + /* Count the newly all-frozen pages for logging. */
>
> AFAIK, we do not use periods in the one line comments. Other than
> that, the patch looks good to me.
Will fix. Thanks
- Melanie
