Question about "component" usage guildline

[Eriko Kurimoto]

Hi build team,

I have a question about when to use "component" in BUILD.gn.

There are two guidelines to decide whether we should use component or not.
One is to avoid having multiple component depending on one single non-component set, and the other is to use component only for medium or larger sized set.

There is a case where these guidelines conflict with each other.

For example, //components/pref_registry which I turned into component just yesterday (CL). It was dependent by multiple components such as "//ash", "//components/onc", "//components/proxy_config", "//components/keyed_service/content" etc... while it only consists of 2 files.

"//components/version_infol" is another such example and there would probably be more.

What do you think is better in such cases?

Personally, I prefer not having the risk of breakage especially because such dup is not detected by CQ (CQ only builds component build but does not test it) so I would like to prioritize the rule described in the "Dependencies between targets" section. But I understand lowering the compilation cost is the main point of component build.

Another option is to keep the mini size set as static_library as long as it has globals or singletons that actually do break on duplication.

I would like to hear your opinions on this.

Thanks,
Eriko

[Nico Weber]

Hi,

It's a good question. I'd say definitely use a component for small targets when it's needed for correctness :)

Do you have a sense for how many of our small targets are depended on by multiple targets?

cc thomasanderson who has worked on this some in the past.

Nico

[Dirk Pranke]

I think in general the first case (dependencies between targets) wins and should win; it's too easy to create build problems if you try to rely on source_sets and static_libraries in a component build. This ends up meaning that things being components are infectious.

-- Dirk

--
You received this message because you are subscribed to the Google Groups "build" group.
To unsubscribe from this group and stop receiving emails from it, send an email to build+un...@chromium.org.
To view this discussion visit https://groups.google.com/a/chromium.org/d/msgid/build/CADZ1XiYcJJjV5TjoZkiDjDXAdLe%3DfzuK4xsQ-aHzXq_3X20QdA%40mail.gmail.com.

[Eriko Kurimoto]

> Do you have a sense for how many of our small targets are depended on by multiple targets?

There are many such cases inside //components that are directly or indirectly depended by components targets.

I checked directories starting from "a" under //components and here are such cases:

- components/access_code_cast/common/

- components/account_id

- components/app_constants

- components/app_launch_prefetch (already marked as component)

So, I would expect it will be around 30 inside //components.

Also, another discussion point is that what we should do for the target with small files and depended by only one component so far.

If we keep them as non-component, it's hard to realize when the component is added to another component build logic in the future.

Thanks,

Eriko

[Kyle Horimoto]

On Thursday, November 7, 2024 at 5:50:59 PM UTC-8 Eriko Kurimoto wrote:

> Do you have a sense for how many of our small targets are depended on by multiple targets?

There are many such cases inside //components that are directly or indirectly depended by components targets.

I checked directories starting from "a" under //components and here are such cases:

- components/access_code_cast/common/

- components/account_id

- components/app_constants

- components/app_launch_prefetch (already marked as component)

So, I would expect it will be around 30 inside //components.

Also, another discussion point is that what we should do for the target with small files and depended by only one component so far.

If we keep them as non-component, it's hard to realize when the component is added to another component build logic in the future.

+1, has there been any consideration for adding presubmit checks to detect when this type of issue occurs?

[Dirk Pranke]

On Fri, Nov 8, 2024 at 9:46 AM Kyle Horimoto <khor...@google.com> wrote:

On Thursday, November 7, 2024 at 5:50:59 PM UTC-8 Eriko Kurimoto wrote:

> Do you have a sense for how many of our small targets are depended on by multiple targets?

There are many such cases inside //components that are directly or indirectly depended by components targets.

I checked directories starting from "a" under //components and here are such cases:

- components/access_code_cast/common/

- components/account_id

- components/app_constants

- components/app_launch_prefetch (already marked as component)

So, I would expect it will be around 30 inside //components.

Also, another discussion point is that what we should do for the target with small files and depended by only one component so far.

If we keep them as non-component, it's hard to realize when the component is added to another component build logic in the future.

+1, has there been any consideration for adding presubmit checks to detect when this type of issue occurs?

Not that I know of. It's not easy to query GN about things in presubmits, as GN has to load the whole build graph before it can answer any questions. This would likely make any presubmit checks a lot slower than I would like.

However, it's an idea worth considering so maybe there's something we can do here that would be feasible.

-- Dirk

[Eriko Kurimoto]

To summarize,

1. Definitely fix the case where the non-component depended by multiple components and it actually duplicates the singleton or globals.

2. Should fix the case  where the non-component depended by multiple components even it's not breaking things for now.

3. As for the non-component depended by one component so far, we can keep it as is if we can have presubmit checks to notify when there will be another component depending on it.

As for 3, it's nice to have a presubmit check. Right now we have a debug build but the tests are not running on it. Can we have presubmit checks for those debug build so that we can minimize the time in total compared to the builders running whole tests?

[Eriko Kurimoto]

Also, we may be able to ignore the source set with header files only such as //base/version_info, //components/channel etc... as it won't be harmful.

[Tomasz Sniatowski]

I wonder if this could be doable using gn metadata, to either collect necessary data at gn gen time or even do the check at gn time.

--

Tomasz Śniatowski

---

From: Eriko Kurimoto <elk...@chromium.org>
Sent: Tuesday, November 12, 2024 06:04
To: Dirk Pranke <dpr...@google.com>
Cc: Kyle Horimoto <khor...@google.com>; build <bu...@chromium.org>; Hidehiko Abe <hide...@chromium.org>; Thomas Anderson <thomasa...@chromium.org>; Nico Weber <tha...@chromium.org>
Subject: Re: Question about "component" usage guildline

 

You don't often get email from elk...@chromium.org. Learn why this is important

This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email. For any questions don't hesitate to reach out to secu...@xperi.com

To view this discussion visit https://groups.google.com/a/chromium.org/d/msgid/build/CAGPAi7hZ9Fkp7zbj80Ha4Z8q_oRHzGnRMXKvQ97HeOUB45bRZQ%40mail.gmail.com.