Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for model module exclusions in native mode #37278

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

manusa
Copy link
Contributor

@manusa manusa commented Nov 23, 2023

Description

Partially addresses fabric8io/kubernetes-client#5592 for Quarkus.

Substitutes the accessor methods at the OpenShiftClientImpl class that optionally link OpenShift model modules.

Users who know beforehand that their application is not going to use certain model types, can exclude some of the model modules to reduce the packaged application size.
However, in native mode, these linked classes prevent the native image from being built when the model modules are excluded.

This PR allows for the exclusion of openshift-model-hive, openshift-model-miscellaneous, openshift-model-operator in native mode, which will already downsize the resulting binary considerably.

Discussed at https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/kubernetes-client.20native.20build.20memory.20usage

Relates to

/cc @jamesnetherton @zakkak

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 23, 2023

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Copy link
Contributor

@zakkak zakkak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@manusa nice work, thanks for the patch.

IMO it's the best thing we can do on the Quarkus side, although the right thing would be to do the splitting you propose in kubernetes-client.

If there is no consensus on the splitting, it will probably be worth the effort to include more exclusions since the numbers are still far from what they used to be. See below for reference numbers:

Before kubernetes client update (i.e. #36312)

    17,374 reachable types   (74.1% of   23,446 total)
   34,042 reachable fields  (71.1% of   47,886 total)
  125,928 reachable methods (67.0% of  188,051 total)
    7,162 types, 1,163 fields, and 43,454 methods registered for reflection
       61 types,    59 fields, and    55 methods registered for JNI access
...
  43.61MB (48.22%) for code area:    92,240 compilation units
  46.42MB (51.33%) for image heap:  465,045 objects and 59 resources
 410.02kB ( 0.44%) for other data
  90.43MB in total

Before this PR (with exclusion hack in the test)

   26,275 reachable types   (93.1% of   28,219 total)
   55,215 reachable fields  (79.9% of   69,072 total)
  271,108 reachable methods (83.3% of  325,379 total)
   16,097 types, 34,196 fields, and 163,511 methods registered for reflection
       61 types,    59 fields, and    55 methods registered for JNI access
        4 native libraries: dl, pthread, rt, z
...
 105.50MB (50.60%) for code area:   236,156 compilation units
 102.61MB (49.21%) for image heap:  820,279 objects and 59 resources
 410.38kB ( 0.19%) for other data
 208.51MB in total

Before this PR (without the exclusion hack in the test)

   35,548 reachable types   (94.4% of   37,641 total)
   75,427 reachable fields  (79.0% of   95,511 total)
  343,186 reachable methods (85.5% of  401,567 total)
   24,759 types, 51,561 fields, and 215,013 methods registered for reflection
       61 types,    61 fields, and    55 methods registered for JNI access
...
 131.64MB (48.15%) for code area:   305,363 compilation units
 141.43MB (51.72%) for image heap:1,024,956 objects and 156 resources
 364.20kB ( 0.13%) for other data
 273.43MB in total

After this PR

   25,042 reachable types   (92.8% of   26,986 total)
   51,559 reachable fields  (78.8% of   65,416 total)
  248,651 reachable methods (82.1% of  302,878 total)
   14,864 types, 30,540 fields, and 145,489 methods registered for reflection
       61 types,    59 fields, and    55 methods registered for JNI access
        4 native libraries: dl, pthread, rt, z
...
   97.16MB (50.48%) for code area:   214,862 compilation units
  94.92MB (49.31%) for image heap:  768,764 objects and 59 resources
 410.17kB ( 0.21%) for other data
 192.48MB in total

@maxandersen
Copy link
Member

nice - fyi, I'm wondering if those inclusions/exclusions can be detected via annotations/type used? that's how we hide lots of this complexity in other quarkus extensions.

i.e. we tend to exclude all by default but then add based on what config/extensions/types/annotations are referenced which we can then deduce at buildtime which part of the model the user is going to need.

@geoand
Copy link
Contributor

geoand commented Nov 23, 2023

I'm wondering if those inclusions/exclusions can be detected via annotations/type used? that's how we hide lots of this complexity in other quarkus extensions

I don't think that can be done here, as the client use is programmatic, not declarative

@zakkak
Copy link
Contributor

zakkak commented Nov 23, 2023

@geoand is this OK to be merged? Are you waiting for more input?

@geoand
Copy link
Contributor

geoand commented Nov 23, 2023

+1 from me

@zakkak zakkak merged commit 63ff7b0 into quarkusio:main Nov 23, 2023
50 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.7 - main milestone Nov 23, 2023
@manusa manusa deleted the feat/model-exclussions branch November 23, 2023 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants