Skip to content

Conversation

@fredj
Copy link
Member

@fredj fredj commented Apr 14, 2016

Reuse the extent and viewHints arrays from the previous frameState to reduce the number of temporary array

*/
ol.View.prototype.getHints = function() {
return this.hints_.slice();
ol.View.prototype.getHints = function(opt_hints) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not opposed to this. But do you see any problem with just returning this.hints_?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure about this; frameState.viewHints is updated in (at least) the animation functions

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I agree we can't reuse the same hints array everywhere. Though I think it is a pity that view hints are specific to a frame instead of a view.

If I ever get around to changing how animation works (so view properties are properly animated and we could finally have a "side-by-side" example that looks nice), then I think we won't need the extra code here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the reminder, #5234

@tschaub
Copy link
Member

tschaub commented Apr 14, 2016

This looks like a good idea to me. I didn't check where else view.getHints() is called, but I wonder if we could just pass around a single array instead of adding the opt_hints arg.

I'm also curious to learn if you are profiling to help identify places where garbage collection can be reduced. Or if you are just identifying optimization opportunities like this by looking through the code.

@fredj
Copy link
Member Author

fredj commented Apr 14, 2016

I'm also curious to learn if you are profiling to help identify places where garbage collection can be reduced. Or if you are just identifying optimization opportunities like this by looking through the code.

It was very manual; I've added breakpoints in size.js, extent.js, ... where the optional value is not provided. For example with :

ol.size.buffer = function(size, buffer, opt_size) {
 ...
};

I've added a conditional breakpoint if opt_size == undefined.

@tschaub
Copy link
Member

tschaub commented Apr 14, 2016

Thanks for the extra detail @fredj. This looks good to merge.

@fredj
Copy link
Member Author

fredj commented Apr 14, 2016

Thanks for the review

@fredj fredj merged commit b7278f1 into openlayers:master Apr 14, 2016
@fredj fredj deleted the reuse_frameState branch April 14, 2016 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants