mapbox / mapbox-maps-ios

Interactive, thoroughly customizable maps for iOS powered by vector tiles and Metal
https://www.mapbox.com/mapbox-mobile-sdk
Other
463 stars 149 forks source link

Snapshotter Causes App Freeze With 100% CPU Load #1196

Open hec72 opened 2 years ago

hec72 commented 2 years ago

Our project makes heavy use of the Snapshotter class in MapboxMaps v 10.4.0.rc-1. Unfortunately, this causes sporadic App freezes with 100% CPU load. Looking at the current call stack always leads back to

Attribution.parse([String]) -> [Attribution]

It seems that using NSMutableAttributedString to convert HTML attribution strings to AttributedStrings causes problems.

This happens even with the Attribution option disabled for the Snapshotter:

let options = MapSnapshotOptions(size: imageSize, pixelRatio: UIScreen.main.scale, showsLogo: false, showsAttribution: false)
let snapshotter = Snapshotter(options: options)
snapshotter.style.uri = .init(rawValue: mapBoxStyleURL)

The current call stack can be seen in the attached Xcode screenshot.

Freeze

ZiZasaurus commented 2 years ago

@hec72 can you provide us with the style and camera you are using?

hec72 commented 2 years ago

Thanks for the quick response!

This is the Style we are using (I altered the name for privacy reasons). I am not sure about the Camera.

mapbox-style.json.zip

ZiZasaurus commented 2 years ago

@hec72 thank you for this information. I noticed that you mentioned this issue occurs sporadically. How often would you say this happens? Additionally, do you experience this freezing when using one of our default styles?

hec72 commented 2 years ago

Ok, so I did a little more testing.

  1. The freeze will occur in approx. 80% of my daily use cases. Sometimes it takes a little longer for it to happen, but currently it seems that it will happen eventually.
  2. The String that is being parsed when the freeze occurs always seems to the the one I pasted below.
  3. When using the default Style .streets as MapView style, the freeze does not seem to occur.
(lldb) po sourceAttributions
▿ 1 element
  - 0 : "<a href=\"https://www.mapbox.com/about/maps/\" target=\"_blank\" title=\"Mapbox\" aria-label=\"Mapbox\" role=\"listitem\">&copy; Mapbox</a> <a href=\"https://www.openstreetmap.org/about/\" target=\"_blank\" title=\"OpenStreetMap\" aria-label=\"OpenStreetMap\" role=\"listitem\">&copy; OpenStreetMap</a> <a class=\"mapbox-improve-map\" href=\"https://www.mapbox.com/contribute/\" target=\"_blank\" title=\"Improve this map\" aria-label=\"Improve this map\" role=\"listitem\">Improve this map</a>"

Additional note: For testing purposes I modified Attribution.parse(_ rawAttributions: [String]) -> [Attribution] to return an empty array and the Freeze does not occur with this modification.

ZiZasaurus commented 2 years ago

@hec72 thank you for this information. So I can better assist you, can you please provide a minimal test case (including the style you are referencing) that reproduces this behavior, as I am unable to reproduce it in my environment.

hec72 commented 2 years ago

The setup our App uses is quite complicated, so I guess it will not be be easy to set up a minimal test case. But I will try and find an example.

In the meantime you might want to take a look at this thread on the Apple Developer Forums. It seems the NSMutableAttributedString API has been causing problems for quite some time now: https://developer.apple.com/forums/thread/115405

macdrevx commented 2 years ago

@evil159 I see 2 possibilities here (there could be others):

  1. As mentioned above, avoid calling Attribution.parse() if showsAttribution is false.
  2. Move the parsing off the main thread. This would require using some other parsing approach since NSAttributedString(data:options:documentAttributes:) docs state:

The HTML importer should not be called from a background thread (that is, the options dictionary includes documentType with a value of html). It will try to synchronize with the main thread, fail, and time out. Calling it from the main thread works (but can still time out if the HTML contains references to external resources, which should be avoided at all costs). The HTML import mechanism is meant for implementing something like markdown (that is, text styles, colors, and so on), not for general HTML import.

https://github.com/Cocoanetics/DTCoreText is BSD-licensed and could offer a path forward if we're willing to take it on as a dependency. We'd need to take a look at how it would impact binary size.