Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests of V8::AdjustAmountOfExternalAllocatedMemory #119

Closed
springmeyer opened this issue Aug 1, 2012 · 6 comments
Closed

tests of V8::AdjustAmountOfExternalAllocatedMemory #119

springmeyer opened this issue Aug 1, 2012 · 6 comments
Labels

Comments

@springmeyer
Copy link
Member

Need tests to ensure proper usage of V8::AdjustAmountOfExternalAllocatedMemory and expected behavior to prevent #116 from regressing.

@springmeyer
Copy link
Member Author

These test could also do timing tests of small batches of allocations vs large batches of cached image/grid allocations to try to get at any scenarios where it is not useful to actually hint to v8 about external memory.

@strk
Copy link
Contributor

strk commented Aug 2, 2012

Another thing I noticed while looking at those calls is that you're advertising more allocations in the copy-constructor, but it wasn't evident that the copy was really copying anything rather than adding a new reference.

I'm still around for a bit because I'm still getting an higher GC cost with node-0.7.9 than with 0.5.11

@springmeyer
Copy link
Member Author

You mean Image::Image(image_ptr this_) ? Its not a copy-ctor but rather an alternative constructor for when images are read off the filesystem.

If you are seeing higher GC cost, definitely just try commenting all usages of V8::AdjustAmountOfExternalAllocatedMemory. The app should work totally fine without this stuff - they are there to help avoid low v8 memory situations when many 1000's of mapnik.Image objects are held in queues.

@strk
Copy link
Contributor

strk commented Aug 2, 2012

Yeah, that's what I meant. All good if it's another constructor.

The cost I'm seeing is likely due to an high memory use, could very well be outside the node-mapnik module (I've seen a global "cache" in tilelive somewhere but could also be in Windshaft or Windshaft-cartodb, don't worry about that here :)

@springmeyer
Copy link
Member Author

Yes tilelive/tilelive-mapnik caches "solid" images now by default to save on encoding. This again, is something that was added with batched rendering in mind, so it may be beneficial to disable it for different serving scenarios.

@springmeyer
Copy link
Member Author

closing, not planning on adding any more tests rather if I do anything I'd remove this code: #368

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants