Skip to content
This repository has been archived by the owner on Feb 23, 2023. It is now read-only.

Measure and potentially fix build time regression #380

Closed
sdeleuze opened this issue Nov 24, 2020 · 5 comments
Closed

Measure and potentially fix build time regression #380

sdeleuze opened this issue Nov 24, 2020 · 5 comments
Assignees
Labels
type: regression A bug that is also a regression
Milestone

Comments

@sdeleuze
Copy link
Contributor

sdeleuze commented Nov 24, 2020

My measurements on webmvc-tomcat sample

  • 0.8.2 - 20.2.0 : 64.8s
  • 0.8.2 - 20.3.0 : 60.9s
  • 0.8.3 - 20.2.0 : 74.7s
  • 0.8.3 - 20.3.0 : 71.8s

So there is a regression between 0.8.2 and 0.8.3 we should identify and remove.

Originally raised via https://spring.io/blog/2020/11/23/spring-native-for-graalvm-0-8-3-available-now#comment-5163901805.

@sdeleuze sdeleuze added the type: regression A bug that is also a regression label Nov 24, 2020
@sdeleuze sdeleuze added this to the 0.9.0 milestone Nov 24, 2020
@sdeleuze sdeleuze changed the title Build time regression Fix build time regression Nov 24, 2020
@sdeleuze sdeleuze self-assigned this Jan 28, 2021
@sdeleuze sdeleuze changed the title Fix build time regression Measure and potentially fix build time regression Feb 3, 2021
@sdeleuze
Copy link
Contributor Author

@aclement I compared both latest 0.9.0-SNAPSHOT and 0.8.2 and I confirm there is a significant regression visible on commandinerunner and webmvc-tomcat samples. Notice this is not specific to bti, the regression happened between 0.8.2 and 0.8.3 and leads to bigger native-image so longer build. Comparing what is included in the image should allow us to identify and maybe fix this regression.

@sdeleuze
Copy link
Contributor Author

I think I have found the root cause and a fix, seems to be related to lack of code removal at build time, I will share more details later.

@sdeleuze
Copy link
Contributor Author

sdeleuze commented Feb 15, 2021

Based on my tests, it seems leveraging the new NativeDetector utility seems to reduce image size and build time by 15%, to this issue should be fixed when #432 will be fixed.

@sdeleuze
Copy link
Contributor Author

Ok I get more evidences, previous finding as just due to oracle/graal#3163 so not relevant.

I did a diff of the classes shipped in the native-image between 0.8.2 (known to compile fast) and master. The difference seems to be due to reflection entries bellow present only in master:
AbstractErrorController
AutoConfigurationImportSelector methods
BasicErrorController methods
CacheType
Compression
DataSize
DateTokenConverter
DefaultMessageCodesResolver$Format
DeserializationFeature
DocumentBuilderFactoryImpl
Encoding
ErrorProperties
GenericWebApplicationContext methods
Http2
HttpStatus
IntegerTokenConverter
JsonAutoDetect
JsonGenerator methods
JsonParser
Jsp
MapperFeature
MediaType
PropertyAccessor
SerializationFeature
Session
SpringApplication
Ssl
Yaml -> see #484

Some could be legit to fix compat, some could be just footprint regression due to hint updates. See attached diff for more details.
diff-0.8.2.txt

aclement added a commit that referenced this issue Feb 22, 2021
This change includes enhancements to the configuration properties
analysis to try and get closer to how 0.8.2 behaved. 0.8.2 didn't
do the right thing for all types of configuration property usage
so now we are more correct it is not surprising we have to
pay a price for that. These changes try to minimize that impact.
There are also some hint adjustments that seem reasonable.

These changes also connect configuration property reflective
access to the build time property checking option. Using that
option can create a further saving in memory/image-size.
@aclement
Copy link
Contributor

Some work done. Some of the new types that are appearing in the diff now are due us being better/more-compatible - in particular they relate to the changes in handling for configuration properties (using getters return types). So what we are doing is more correct but we are paying a heavy price for properties that will never be bound. I've enhanced that property code again, it is smarter now, which helps. I also wired that code up to the build-time-property-checking flags for the first time (defaults to off but you can turn it on) - previously that was only wired to the @ConditionalOnProperty checks.

I've stopped using J11 for this kind of analysis, the unpredictable numbers drive me crazy. So comparing J8 below.

Comparing 0.8.2 and 0.9.0: (first are 0.8.2, second are 0.9.0 and third are 0.9.0 with build time property checking turned on):

commandlinerunner
Build Time:  47.1s > 45.8s > 46.5s
RSS Memory: 23.1 > 21.8M > 21.6M
Image Size: 20.7M > 20.9M > 20.9M

webmvc-tomcat
Build Time: 70.2s > 76.7s > 76.6s
RSS Memory: 52.0M > 50.1M > 48.2M
Image Size: 43.1M > 44.8M > 43.5M

Some of this included hint tweaks so I need to see a clean samples run on the CI to verify nothing is really broken. I'll do a bit more on it this week whilst these changes bed in.

@sdeleuze sdeleuze removed their assignment Feb 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: regression A bug that is also a regression
Development

No branches or pull requests

2 participants