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

Use OkHTTP to implement HTTPContext on Android #2033

Merged
merged 1 commit into from
Aug 12, 2015
Merged

Conversation

ljbade
Copy link
Contributor

@ljbade ljbade commented Aug 11, 2015

Closes #823

@ljbade
Copy link
Contributor Author

ljbade commented Aug 11, 2015

This is a pretty big merge with lots of new JNI code.

@kkaefer @jfirebaugh Do you mind giving this a review?

@jfirebaugh
Copy link
Contributor

I'm not really familiar with any of this code. @incanus or @bleege are probably better reviewers.

@mb12
Copy link

mb12 commented Aug 11, 2015

1.) Can you also share improvements in APK file size and any changes in performance w.r.t runtime and memory?
2.) Also do tile requests for Android do a lookup in sqlite cache before going over the network? Will this also be moved to managed code?

@ljbade ljbade added the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Aug 11, 2015
@ljbade
Copy link
Contributor Author

ljbade commented Aug 11, 2015

@mb12 In this branch everything still goes via sqlite cache. Can't see why we can't use Java side caching.

Does OkHTTP have support for that?

I haven't done checks of the APK size or performance comparisons yet.

@ljbade
Copy link
Contributor Author

ljbade commented Aug 11, 2015

Going to collect here before/after .aar file sizes:

For just ARM 32bit lib:

  • Before: 3.0MB
  • After: 2.0MB

All libs:

  • Before:
  • After:

@ljbade
Copy link
Contributor Author

ljbade commented Aug 11, 2015

@kkaefer Do you know a good way to quickly benchmark the tile fetch performance with these changes?

@ljbade ljbade merged commit 0cbe7e0 into master Aug 12, 2015
@ljbade ljbade deleted the android-okhttp branch August 12, 2015 04:59
@1ec5 1ec5 removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Nov 9, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants