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

fix: add stability configuration file to the core maps-compose projec… #517

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

bubenheimer
Copy link
Contributor

…t. The currently configured types are all the immutable data types from com.google.android.gms.maps.model that are currently used in android-maps-compose:maps-compose. These types will now be considered @stable by the compiler. Making immutable data types @stable can be a more effective measure than relying on strong skipping, as stable types can use structural equality, whereas strong skipping merely relies on referential equality, as of today.

This commit also makes compose compiler report generation available via command-line options (-PcomposeCompilerMetrics=true, -PcomposeCompilerReports=true)

Fixes #152

…t. The currently configured types are all the immutable data types from com.google.android.gms.maps.model that are currently used in android-maps-compose:maps-compose. These types will now be considered @stable by the compiler. Making immutable data types @stable can be a more effective measure than relying on strong skipping, as stable types can use structural equality, whereas strong skipping merely relies on referential equality, as of today.

This commit also makes compose compiler report generation available via command-line options (-PcomposeCompilerMetrics=true, -PcomposeCompilerReports=true)

Fixes googlemaps#152
@bubenheimer
Copy link
Contributor Author

@kikoso are you open to reviewing this PR?

@bubenheimer
Copy link
Contributor Author

@wangela can this PR receive a review, please?

@kikoso
Copy link
Collaborator

kikoso commented Apr 15, 2024

Hi @bubenheimer . Thanks for the PR, and for the patience while getting this reviewed.

This should improve the stability and performance. Is there a reason why you would not include the other types in the model folder?

@bubenheimer
Copy link
Contributor Author

bubenheimer commented Apr 15, 2024

@kikoso The types in model are not generally immutable, which commonly would make them non-stable for Compose purposes. They would be covered by the upcoming strong skipping changes for handling non-stable types in Compose, which is a little less powerful than what we can achieve with stability configuration files.

I've validated for the types in the stability configuration file that they are immutable. These are all the Maps SDK model types currently in use in the maps-compose library.

I did not look at what's being used in maps-compose-utils, and did not provide a stability configuration file there, as I don't use that lib, not familiar with it. It can be done independently of this PR.

@bubenheimer
Copy link
Contributor Author

@kikoso Actually, maps-compose uses some mutable types from model as well (e.g. Marker), but they need to be handled via strong skipping changes, not stability configuration file, for the the reason I gave above.

@kikoso
Copy link
Collaborator

kikoso commented Apr 16, 2024

Thanks @bubenheimer , that makes sense. We will wait until the strong skipping mode exits its experimental phase before adopting it. However, this already represents an improvement in terms of performance.

@dkhawk dkhawk merged commit bfe95dd into googlemaps:main Apr 16, 2024
8 of 9 checks passed
googlemaps-bot pushed a commit that referenced this pull request Apr 16, 2024
## [4.3.4](v4.3.3...v4.3.4) (2024-04-16)

### Bug Fixes

* add stability configuration file to the core maps-compose project. The currently configured types are all the immutable data types from com.google.android.gms.maps.model that are currently used in android-maps-compose:maps-compose. These types will now be considered @stable by the compiler. Making immutable data types @stable can be a more effective measure than relying on strong skipping, as stable types can use structural equality, whereas strong skipping merely relies on referential equality, as of today. ([#517](#517)) ([bfe95dd](bfe95dd)), closes [#152](#152)
@googlemaps-bot
Copy link
Contributor

🎉 This PR is included in version 4.3.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Performance penalty throughout API from LatLng usages
5 participants