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

~Actor<O> is blocking #2644

Closed
kkaefer opened this issue Oct 16, 2015 · 7 comments
Closed

~Actor<O> is blocking #2644

kkaefer opened this issue Oct 16, 2015 · 7 comments
Labels
archived Archived because of inactivity Core The cross-platform C++ core, aka mbgl performance Speed, stability, CPU usage, memory usage, or power usage

Comments

@kkaefer
Copy link
Member

kkaefer commented Oct 16, 2015

When canceling a WorkTask, we block until that task has completed its entire invocation. This isn't something we should do. You can make this behavior more visible when adding a sleep(4) in a worker task, (e.g. in TileWorker::parse()). Panning fast now stalls heavily, as partial work requests are getting canceled.

/cc @jfirebaugh

@kkaefer kkaefer changed the title WorkTask blocks cancellation WorkTask cancelation is blocking Oct 16, 2015
@jfirebaugh
Copy link
Contributor

This is by design. WorkTasks typically have references to objects which need to outlive the task, whose owner is in turn owning the task. If the task isn't canceled, then these lifetime invariants will not hold: the objects will be destroyed along with their owner, while the work task will continue running with dangling references.

@jfirebaugh
Copy link
Contributor

Worker tasks should be designed to check a flag in inner loops and exit early if the flag indicates the task has been cancelled. Most do; the exception I see is SymbolBucket, which has no early exit for the loops in needsDependencies and addFeatures.

@tmpsantos
Copy link
Contributor

This is by design.

And I like the design here because protects us from bugs that are very difficult to debug that haunted us in the past. On #3636 suggested a special deleter for TileData that if turns out to be needed somewhere else we could make it more generic and call it NonBlockingDeleter and have a method like ::willBlockOnDestructor() to postpone the deletion on objects with WorkRequests.

@kkaefer
Copy link
Member Author

kkaefer commented Apr 15, 2016

This is resurfacing: When parsing tiles, we're checking a flag after parsing every feature whether we still need the tile. However, our landuse layer, in particular on zoom level 7, contains some very complex features that take > 1 second to parse in their call to clipper + libtess2. These tasks are not interruptible. We should do two things:

  1. Make tile parsing cancellation non-blocking. This means that parsing a tile must not access resources shared with the main thread or other threads.
  2. Refactor our landuse layer to avoid single polygons that are too complex

/cc @mourner

@1ec5 1ec5 added performance Speed, stability, CPU usage, memory usage, or power usage Core The cross-platform C++ core, aka mbgl labels Apr 21, 2016
@jfirebaugh
Copy link
Contributor

As of #6337 there's no cancellation of tasks, but destroying the actor can still block. We need #667 in order to remove that.

@jfirebaugh jfirebaugh changed the title WorkTask cancelation is blocking ~Actor<O> is blocking Sep 16, 2016
@jfirebaugh
Copy link
Contributor

It should be safe to change this now.

@stale stale bot added the archived Archived because of inactivity label Nov 7, 2018
@stale
Copy link

stale bot commented Nov 27, 2018

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this as completed Nov 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived Archived because of inactivity Core The cross-platform C++ core, aka mbgl performance Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

No branches or pull requests

4 participants