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

From: "Alex Bennée" <alex.bennee@linaro.org>
To: Sven Schnelle <svens@stackframe.org>
Cc: Alexandre Iooss <erdnaxe@crans.org>,
	 Mahmoud Mandour <ma.mandourr@gmail.com>,
	 Pierrick Bouvier <pierrick.bouvier@linaro.org>,
	qemu-devel@nongnu.org,  deller@gmx.de
Subject: Re: [PATCH v3 01/12] util/log: convert debug_regions to GList
Date: Mon, 04 Mar 2024 17:58:48 +0000	[thread overview]
Message-ID: <87il21yj3b.fsf@draig.linaro.org> (raw)
In-Reply-To: <87bk7uue47.fsf@t14.stackframe.org> (Sven Schnelle's message of "Mon, 04 Mar 2024 17:59:52 +0100")

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