Re: [PATCH v3 01/12] util/log: convert debug_regions to GList

[Alex Bennée]

Sven Schnelle <svens@stackframe.org> writes:

> Alex Bennée <alex.bennee@linaro.org> writes:
>
>> Sven Schnelle <svens@stackframe.org> writes:
>>
>>> In preparation of making the parsing part of qemu_set_dfilter_ranges()
>>> available to other users, convert it to return a GList, so the result
>>> can be used with other functions taking a GList of struct Range.
>>
>> Why do we need to convert it to a Glist? When I originally wrote the
>> dfilter code I deliberately chose GArray over GList to avoid bouncing
>> across cache lines during the tight loop that is checking ranges. It's
>> not like we can't pass GArray's to plugins as well?
>
> Looking at the code again, i remember that the reason for choosing GList
> was that the other range matching function all take GList's - having
> some functions take GArray's, and some not would be odd.

Ahh I see. There wasn't intended to be much of a relationship between
the dfilter code and the range code apart from the fact I re-used the
Range typedef to avoid duplication.

> So i wonder
> whether we should convert all of those functions to GArray? (if
> possible, i haven't checked)

I think that would depend on access patterns and flexibility. For the
dfilter there is no particular need for sorting and the principle
operation is a sequence of lookups. For the other use cases keeping a
sorted list and quick insertion might be more useful.

While its tempting to go on a wider re-factoring I would caution against
it so close to softfreeze.

> What do you think?

Lets stick to the dfilter case and worry about wider clean-ups later. As
Richard points out it might be the interval tree makes more sense for
some of these things.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro