[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gentoo-dev
Subject:    Re: [gentoo-dev] [PATCH 1/7] eclass/nuget.eclass: introduce new eclass
From:       Florian Schmaus <flow () gentoo ! org>
Date:       2023-07-31 14:20:02
Message-ID: 04136d83-6043-848f-a2ec-a111bec19938 () gentoo ! org
[Download RAW message or body]

[Attachment #2 (multipart/mixed)]

[Attachment #4 (multipart/mixed)]

[Attachment #6 (text/plain)]

On 31/07/2023 15.53, Michał Górny wrote:
> On Mon, 2023-07-31 at 12:49 +0200, Florian Schmaus wrote:
>> On 31/07/2023 11.32, Sam James wrote:
>>>
>>> Florian Schmaus <flow@gentoo.org> writes:
>>>
>>>> [[PGP Signed Part:Undecided]]
>>>> On 31/07/2023 07.02, Michał Górny wrote:
>>>>> On Sun, 2023-07-30 at 22:19 +0200, Florian Schmaus wrote:
>>>>>> Which problem are we solving by moving away from this towards a slightly
>>>>>> more verbose construct?
>>>>> The problem was that cargo.eclass ebuilds were taking significant
>>>>> time
>>>>> during cache regeneration and slowing down tools noticeably.  No fancy
>>>>> loops required, contrary to your great theory.
>>>>
>>>> Removing the $()/fork from go-modules.eclass reduced the source time
>>>> of a package from 2400 milliseconds to 236 milliseconds.
>>>>
>>>> Changing, for example net-p2p/arti-1.1.6, to use _cargo_set_crate_uris
>>>> reduces the source time from 44 milliseconds to 24 milliseconds.
>>>>
>>>> That is a win in relative reduction, but absolute its just 20
>>>> milliseconds. Cache regeneration is an embarrassingly parallel
>>>> problem. Therefore such a reduction should not matter much, assuming
>>>> you have some parallelism on the hardware level.
>>>
>>> Consistency matters
>>
>> Sure, I would be in favor of consistently using $(foo_uris).
>>
>> Especially since the performance gains of the variable-setting approach
>> are even lower than I first assumed. The cargo.eclass runs the function
>> that computes CARGO_CRATE_URIS now twice, which adds significantly more
>> overhead than the fork of $(foo_uris). See my patch to the ML.
>>
> 
> So, to summarize, your point is that after you've ignored the original
> thread

I can not say that I have actively ignored the thread. I am simply not 
aware of such a thread where we discussed that the variable-setting 
pattern has to be used instead of $(foo_uris).

I am sorry, I must have missed it. And it appears that my search-foo is 
not strong enough to dig it up. Could you please point me to the thread?

Or, are you maybe referring to the thread with your cargo.eclass patches 
from June, 16th? That seemed to be only about cargo.eclass and not about 
a tree-wide policy.


> and we've actually started switching stuff to ${xxx}, we should
> reopen the discussion and start moving everything back to $(xxx),

I never demanded that anything should be moved back to $(foo_uris). It's 
up to the eclass maintainer to decide which reasoning the follow and how 
they weight the arguments.


> even
> though you've proven yourself that it's less optimal ("but only
> a little!") and because... you prefer it? 

Optimality is not one-dimensional. Speed is not everything.

The $(foo_uri) pattern has slightly less code complexity and is slightly 
better readable. Moreover, given that the justification for moving away 
from it is negligible speedup, I prefer $(foo_uri).

Moving from $(foo_uri) to the variable-setting pattern ${foo_uri} appear 
to be a solution to a non-existing problem.

I have only expressed my opinion. I think that the variable-setting 
pattern is somewhat disadvantageous in this case.

However, this is not a hill for me to die on.


> Yes, that certainly makes
> sense.  It's surely a great way to run a distro is to undo optimizations
> 6 weeks later because you liked the old variant better.

Again, nobody asked for any undoing.

- Flow


["OpenPGP_0x8CAC2A9678548E35.asc" (application/pgp-keys)]
["OpenPGP_signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]