Re: [PATCH 4/4] mark pseudo users as deleted instead of removing them

[Luc Van Oostenryck <luc.vanoostenryck@gmail.com>]

On Fri, Aug 4, 2017 at 11:14 PM, Christopher Li <sparse@chrisli.org> wrote:
> On Fri, Aug 4, 2017 at 5:03 PM, Luc Van Oostenryck wrote:
>>> I am not sure we are on the same page. Only 2 macro need to be updated:
>>> DO_FOR_EACH_PTR, DO_FOR_EACH_REVERSE.
>>
>> It's not for the number of macros defined.
>> It's about the code size & the run-time effect on the iteration of
>> the other lists. It seems simple enough but it would add two load-test-branch
>
> Because the rm is near the nr, which get visit every loop iteration.
> From cache line point of view, the ptrlist->rm will not cause extra cache load.
> The second test and load will not matter for most of the ptrlist any way.

Yes, we agree.

> I am willing to bet even my test-ptrlist benchmark will not see much a
> difference.

It will depend on how micro the benchmark is but indeed on real code,
most probably it won't make any measurable difference.

>> to every such macro invocation and these macros are already quite heavy.
>
> The flip side is if some one forget to check that, it will be very
> bad. How do you
> make sure you add the check to every single one of the loop iterator?

Oh, I agree on this one but the same argument when pushed further
can lead to absurd situations. We're not there though.

> I can take patch as it is. It is only a suggestion. No deal breaker.

Frankly, I don't know.

I never liked to have to add these checks a bit everywhere,
that's why I called it 'not very pretty'.
My inclination would be to create a variant FOR_EACH_PTR_NULL(),
this can be done without duplicating the real macro DO_...
but then you can as easily forgot to call this variant that you could
forgot to add the test locally (adding some typing could help here though).
On the other hand, adding these tests to all list iteration code when
not needed looks bad.

OK, since the run-time impact will be low enough let's choose the
solution with the smallest (source) code impact.
New version on the way.

-- Luc