use a pool of images to reduce re-allocation overhead #50

Closed
springmeyer opened this Issue Nov 12, 2012 · 8 comments

Comments

Projects
None yet
2 participants
@springmeyer
Member

springmeyer commented Nov 12, 2012

For export jobs that attempt to maximize machine resources in order to render a tile cache as fast as possible, the allocation and cleanup of mapnik.Image objects shows up in profiling traces. While image allocation seems like it should be of trivial cost compared to data access and rendering, the sheer number of images allocated (per metatile) + the nature of v8 garbage deferred collection means that we may be able to speed up renders by pooling images and re-using them.

Issues to tackle:

  • _DONE, YES_: is there a clear benefit to pooling images and re-using images (after clearing their data) to reduce the number of allocations per render?
  • _DONE, YES_ can both images and mapnik.Grid objects still be treated the same here?
  • _TODO:_ is there a benefit to making allocation / cleanup / clearing of images operate in the threadpool (be async)?
  • _TODO:_ is using V8::AdjustAmountOfExternalAllocatedMemory in the main thread creating a bottleneck?
  • _TODO:_ a reasonable api for releasing, automatically, or via api call, the image pool
@springmeyer

This comment has been minimized.

Show comment Hide comment
@springmeyer

springmeyer Nov 12, 2012

Member

work underway in branch at master...image-pool

Member

springmeyer commented Nov 12, 2012

work underway in branch at master...image-pool

@yhahn

This comment has been minimized.

Show comment Hide comment
@yhahn

yhahn Nov 12, 2012

Member

FYI the render that was hanging with image-pool succeeded without problems using master. May be worth trying to reduce a simpler test case and try to reproduce the hang once the other TODOs are out of the way?

Member

yhahn commented Nov 12, 2012

FYI the render that was hanging with image-pool succeeded without problems using master. May be worth trying to reduce a simpler test case and try to reproduce the hang once the other TODOs are out of the way?

@springmeyer

This comment has been minimized.

Show comment Hide comment
@springmeyer

springmeyer Nov 12, 2012

Member

Have you ever seen any rendering errors in the logs with runs that did not hang? I presume not, but I found and fixed this bug that would foreseeably cause a hang if a rendering error happened and the pool was not released.

I have progress on code to test the other todo's and then returning to try to reduce the test I think would be a solid plan.

Member

springmeyer commented Nov 12, 2012

Have you ever seen any rendering errors in the logs with runs that did not hang? I presume not, but I found and fixed this bug that would foreseeably cause a hang if a rendering error happened and the pool was not released.

I have progress on code to test the other todo's and then returning to try to reduce the test I think would be a solid plan.

@springmeyer

This comment has been minimized.

Show comment Hide comment
@springmeyer

springmeyer Nov 12, 2012

Member

mapnik/mapnik#1571 is done, which sets me up for exposing node-mapnik methods for clearing, for re-use, both mapnik.Image and mapnik.Grid objects. It is grids, in particular, that I think may need to be cleaned up in the thread pool as they cache lots of shared pointers in memory (refs mapnik/node-mapnik#93).

Member

springmeyer commented Nov 12, 2012

mapnik/mapnik#1571 is done, which sets me up for exposing node-mapnik methods for clearing, for re-use, both mapnik.Image and mapnik.Grid objects. It is grids, in particular, that I think may need to be cleaned up in the thread pool as they cache lots of shared pointers in memory (refs mapnik/node-mapnik#93).

springmeyer pushed a commit to mapnik/node-mapnik that referenced this issue Nov 12, 2012

springmeyer pushed a commit that referenced this issue Nov 12, 2012

@springmeyer

This comment has been minimized.

Show comment Hide comment
@springmeyer

springmeyer Nov 12, 2012

Member

refactored work in e0e089c. Now images and grids can be treated the same in the api. Next up to test various other TODO's for both images and grids.

Member

springmeyer commented Nov 12, 2012

refactored work in e0e089c. Now images and grids can be treated the same in the api. Next up to test various other TODO's for both images and grids.

@springmeyer

This comment has been minimized.

Show comment Hide comment
@springmeyer

springmeyer Mar 28, 2013

Member

stil on my todo list to get this integrated into master - all profiling on os x shows that allocation can commonly take up 2-5% of the main loop's cpu time and this avoids that.

Member

springmeyer commented Mar 28, 2013

stil on my todo list to get this integrated into master - all profiling on os x shows that allocation can commonly take up 2-5% of the main loop's cpu time and this avoids that.

@springmeyer

This comment has been minimized.

Show comment Hide comment
@springmeyer

springmeyer Mar 28, 2013

Member

and up to or greater than 30% for very simple, fast rendering cases like the default project in TileMill.
Screen Shot 2013-03-28 at 11 33 56 AM

Member

springmeyer commented Mar 28, 2013

and up to or greater than 30% for very simple, fast rendering cases like the default project in TileMill.
Screen Shot 2013-03-28 at 11 33 56 AM

@springmeyer

This comment has been minimized.

Show comment Hide comment
@springmeyer

springmeyer Jul 25, 2014

Member

still a worthy idea, but not actionable since effort is mainly over in tilelive-vector and tilelive-bridge now instead of this module.

Member

springmeyer commented Jul 25, 2014

still a worthy idea, but not actionable since effort is mainly over in tilelive-vector and tilelive-bridge now instead of this module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment