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

WorkTasks: destruct data before invoking callback #3711

Merged
merged 1 commit into from
Jan 28, 2016

Conversation

kkaefer
Copy link
Member

@kkaefer kkaefer commented Jan 27, 2016

This is a short term fix for #3636. The reason for the hangs we were seeing was because the following happened:

  • Work task completes, calls callback, which schedules the after callback on the main thread
  • After callback was invoked and tried to cancel the work task first thing, which blocks on the mutex
  • On the worker thread, we were destructing the arguments (in particular the VectorTile object) after we've called the callback in the first step; on slow devices, this could take 20-100ms
  • Worker thread releases mutex, then main thread can finally acquire it to mark the task as canceled

This patch fixes this with the following changes:

  • Move work task arguments from the tuple into the actual function, so they can be destructed there before the callback gets fired
  • Actually destruct the arguments by either resetting, or moving the data into the invocation where it will get destructed

@kkaefer kkaefer self-assigned this Jan 27, 2016
@1ec5 1ec5 added the performance Speed, stability, CPU usage, memory usage, or power usage label Jan 27, 2016
@@ -37,7 +37,7 @@ TileWorker::~TileWorker() {
}

TileParseResult TileWorker::parseAllLayers(std::vector<std::unique_ptr<StyleLayer>> layers_,
const GeometryTile& geometryTile,
std::unique_ptr<const GeometryTile> geometryTile,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by moving the entire unique_ptr, we ensure that it gets destructed when this function returns.

@kkaefer
Copy link
Member Author

kkaefer commented Jan 27, 2016

With this fix, we almost never wait for WorkTask cancelations (= immediate lock acquisition), and if we do, it's only for a very short period of time.

@jfirebaugh
Copy link
Contributor

👍

@1ec5 1ec5 added this to the ios-v3.1.0 milestone Jan 27, 2016
@kkaefer kkaefer force-pushed the 3711-destruct-worktask-data-before-callback branch from 4b47edf to e2a3c02 Compare January 28, 2016 09:52
@kkaefer kkaefer merged commit e2a3c02 into master Jan 28, 2016
@kkaefer kkaefer deleted the 3711-destruct-worktask-data-before-callback branch January 28, 2016 14:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
performance Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants