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

Update to android_system_properties v0.1.5 #56

Merged
merged 2 commits into from Aug 30, 2022

Conversation

Kijewski
Copy link
Collaborator

The new release makes AndroidSystemProperties send + sync, so the
initialization can be cached. This makes the stress test execute in

0m00.61s real     0m04.00s user     0m00.02s system

Before it was

1m37.25s real     1m06.81s user     0m57.95s system

I.e. it's now 9786 % faster. :)

The new release makes `AndroidSystemProperties` send + sync, so the
initialization can be cached. This makes the stress test execute in

```text
0m00.61s real     0m04.00s user     0m00.02s system
```

Before it was

```text
1m37.25s real     1m06.81s user     0m57.95s system
```

I.e. it's now 9786 % faster. :)
@Kijewski Kijewski force-pushed the pr-android_system_properties-0_1_5 branch from 37859f7 to 5c5badb Compare August 30, 2022 11:05
@astraw
Copy link
Member

astraw commented Aug 30, 2022

I don't think we should cache results from the OS, as this would prevent getting an updated time zone in a long running process. With mobile OSes like Android in particular I guess this is an important issue. If better performance is needed, the user can cache themselves.

@Kijewski
Copy link
Collaborator Author

Kijewski commented Aug 30, 2022

This does not cache the result, only the "getter". We do the same already in the windows implementation. The getter is still called in every invocation.

@astraw astraw merged commit 299bff3 into strawlab:main Aug 30, 2022
@Kijewski Kijewski deleted the pr-android_system_properties-0_1_5 branch August 30, 2022 13:48
@astraw
Copy link
Member

astraw commented Aug 30, 2022

Thanks for the clarification. Yes, now that my coffee has kicked in, I see that, too. This looks good, so I merged it and published as 0.1.47.

src/tz_android.rs Show resolved Hide resolved
src/tz_android.rs Show resolved Hide resolved
@lopopolo
Copy link
Collaborator

Sorry I should have clarified with my review. My comments are only nits. Otherwise looks good to me.

@lopopolo lopopolo added the Tier-2 Rust Tier-2 platform label Oct 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tier-2 Rust Tier-2 platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants