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

Overallocate geomBuffer. #941

Merged
merged 2 commits into from
Oct 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
- Coordinate transforms on flat arrays are now faster (#939)
- `convertColor` is memoized to speed up repeated calls (#936)
- All features have a `featureType` property (#931)
- When changing geometry sizes, buffers are reallocated less (#941)

### Changes
- Removed the dependency on the vgl module for the `object` and `timestamp` classes (#918)
Expand Down
28 changes: 24 additions & 4 deletions src/util/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -343,17 +343,37 @@ var util = {
* @param {vgl.geometryData} geom The geometry to reference and modify.
* @param {string} srcName The name of the source.
* @param {number} len The number of elements for the array.
* @param {number} [allowLarger=0.2] If the existing buffer is larger than
* requested, don't reallocate it unless it exceeds the size of
* `len * (1 + allowLarger)`.
* @param {number} [allocateLarger=0.1] If reallocating an existing buffer,
* allocate `len * (1 + allocateLarger)` to reduce the need to reallocate
* on subsequent calls. If this is the first allocation (the previous
* size was 0), `len` is allocated.
* @returns {Float32Array} A buffer for the named source.
* @memberof geo.util
*/
getGeomBuffer: function (geom, srcName, len) {
var src = geom.sourceByName(srcName), data;
getGeomBuffer: function (geom, srcName, len, allowLarger, allocateLarger) {
allowLarger = allowLarger === undefined ? 0.2 : allowLarger;
allocateLarger = allocateLarger === undefined ? 0.1 : allocateLarger;
var src = geom.sourceByName(srcName),
data = src.data(),
allow = Math.floor((allowLarger + 1) * len);

data = src.data();
if (data instanceof Float32Array && data.length === len) {
/* If the current buffer is either the length we want or no larger than a
* factor of allowBigger more in size, just return it. */
if (data instanceof Float32Array && (data.length === len || (data.length >= len && data.length <= allow))) {
return data;
}
data = new Float32Array(len);
/* If we need to allocate a new buffer (smaller or larger), and we have an
* existing, non-zero-length buffer, allocate a larger than needed buffer.
* Add an extra factor of allocateLarger. */
var allocate = len;
Copy link
Member

Choose a reason for hiding this comment

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

why do we care for power of two?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We might not. In general, many memory allocation schemes have different pools that change when a power-of-two boundary is crossed. I don't actually know if Firefox or Chrome do (they used to, but they may no longer do so). By choosing a power-of-two when convenient, it is possible that we will use the memory pool for smaller objects in preference to the larger. The optimization is a guess -- if the buffer gets reallocated bigger, then the smaller size is wasted. If it doesn't get bigger, then we might gain an efficiency.

I can take it out if you'd prefer.

Copy link
Member

Choose a reason for hiding this comment

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

I can take it out if you'd prefer.

I have a slight preference to take it out since this behavior is not documents in the 'user documentation' plus it complicates things further without clear benefit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

if (data instanceof Float32Array && data.length && len && allocateLarger > 0) {
allocate = Math.floor((allocateLarger + 1) * len);
}
data = new Float32Array(allocate);
src.setData(data);
return data;
},
Expand Down