Skip to content
This repository has been archived by the owner on May 9, 2018. It is now read-only.

Converted to ARC #105

Closed
wants to merge 1 commit into from
Closed

Converted to ARC #105

wants to merge 1 commit into from

Conversation

alpascual
Copy link

Migrated to ARC

@incanus
Copy link
Contributor

incanus commented Aug 29, 2012

Thanks for this. I am tracking Outdooractive/route-me#78 upstream for this, so I will likely not move before they do. I'll leave this open in the meantime.

@incanus
Copy link
Contributor

incanus commented Nov 2, 2012

Still in a holding pattern on this for now.

@incanus
Copy link
Contributor

incanus commented Feb 5, 2013

Starting work on this again in https://github.com/mapbox/mapbox-ios-sdk/tree/arc

Sorry @alpascual for sitting on this so long. I just did it again from scratch since many things have changed. Feel free to correct any problems that you can see. I will be trying to keep this in sync with https://github.com/mapbox/mapbox-ios-sdk/tree/develop until such time as we switch over to it as primary.

@sobri909
Copy link

@incanus I've just run into another crash that wouldn't exist in an ARC world, and am considering switching to using the ARC branch instead of release. It looks like you're tracking it fairly close to develop. What's your confidence level in this branch? You recommend, or should I stick with release? Thanks.

@incanus
Copy link
Contributor

incanus commented Feb 22, 2013

I'm feeling good about ARC and could use some testing there. ARC migration aside, it's based on develop as you say and that's nearing a release, so functionality-wise I think it's good. I haven't seen any problems/leaks/etc. with ARC yet and the migration was fairly straightforward but like I say, testing is welcome.

@sobri909
Copy link

Thanks @incanus. So far the only quirk I've run into is a blocking network load during initWithCoder: that causes a ~1 sec delay during viewDidLoad. I can't immediately see why it happens in this branch but not release, as the line in question is near identical.

I've worked around it simply by having initWithCoder: pass in a nil tile source. But that's possibly not desirable for the trunk.

@incanus
Copy link
Contributor

incanus commented Feb 25, 2013

I'll have a look at this. The branded… business is a way to allow the dev to set a custom user-agent that touches all sorts of network requests, so maybe there's something up with that.

@incanus
Copy link
Contributor

incanus commented Feb 26, 2013

I fixed a problem with that category method anyway in f9b8a93. Does that fix what you are seeing? I've merged it over to arc.

@incanus
Copy link
Contributor

incanus commented Feb 27, 2013

The arc branch is following develop pretty closely right now. Testers welcome.

@sobri909
Copy link

@incanus It's still blocking the main thread for a second or more, depending on network response times.

I've spotted the difference now. release uses an instance of RMOpenStreetMapSource in initWithCoder:, but develop uses RMMapBoxSource. RMOpenStreetMapSource has effectively nothing in its init process, but RMMapBoxSource has the synchronous network request in its init process.

So the issue is that RMMapBoxSource is doing a synchronous network request during init.

@incanus
Copy link
Contributor

incanus commented Feb 28, 2013

Ah, sorry, I thought this was clear. Yes, RMMapBoxSource can block if you are using just a map ID or fetching a remote TileJSON to configure the tile source (which are basically the same thing -- a map ID gets inserted into a URL template to fetch TileJSON). I guess this could be worked around by asynchronously fetching this, but it contains info such as the min and max zoom, so things get sticky. You can pass a local URL to TileJSON that you've already got to avoid this; however, this is not doable from a XIB, nor initWithCoder:.

I wonder if there's a way to configure a tile source via an outlet in IB by filling out a text field or something. I imagine that would require an IB plugin. Any ideas?

@incanus
Copy link
Contributor

incanus commented Mar 4, 2013

Merged arc into develop in preparation for release.

@incanus incanus closed this Mar 4, 2013
@sobri909
Copy link

sobri909 commented Mar 5, 2013

@incanus I've never had any luck with providing custom variables in IB, although I've admittedly not tried hard.

My instinct is to say that an init should never block, and should return as quickly as possible. But in this case I don't have a decent workaround to offer.

For my specific case, there's nothing stopping me moving the map creation to code rather than IB, although that still leaves initWithCoder: in a not ideal state for others.

@incanus
Copy link
Contributor

incanus commented Mar 5, 2013

Ok, I've opened a separate issue for this to dive into later. Thanks for the info.

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.

None yet

3 participants