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

Add support for encoding TypedArrays as primitive objects for serialization #2911

Closed
wants to merge 11 commits into from
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
"@plotly/d3-sankey": "^0.5.0",
"alpha-shape": "^1.0.0",
"array-range": "^1.0.1",
"base64-arraybuffer": "^0.1.5",
archmoj marked this conversation as resolved.
Show resolved Hide resolved
"canvas-fit": "^1.5.0",
"color-normalize": "^1.0.3",
"color-rgba": "^2.0.0",
Expand Down
69 changes: 66 additions & 3 deletions src/lib/coerce.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ var nestedProperty = require('./nested_property');
var counterRegex = require('./regex').counter;
var DESELECTDIM = require('../constants/interactions').DESELECTDIM;
var wrap180 = require('./angles').wrap180;
var isArrayOrTypedArray = require('./is_array').isArrayOrTypedArray;
var isArray = require('./is_array');
var isArrayOrTypedArray = isArray.isArrayOrTypedArray;
var isTypedArray = isArray.isTypedArray;
var isPrimitiveTypedArrayRepr = isArray.isPrimitiveTypedArrayRepr;
var b64 = require('base64-arraybuffer');

exports.valObjectMeta = {
data_array: {
Expand All @@ -33,8 +37,18 @@ exports.valObjectMeta = {
otherOpts: ['dflt'],
coerceFunction: function(v, propOut, dflt) {
// TODO maybe `v: {type: 'float32', vals: [/* ... */]}` also
if(isArrayOrTypedArray(v)) propOut.set(v);
else if(dflt !== undefined) propOut.set(dflt);
if(isArrayOrTypedArray(v)) {
propOut.set(v);
} else if(isPrimitiveTypedArrayRepr(v)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, the {dtype: '', data: ''} -> typed array conversion should happen during the calc step. More precisely somewhere here:

ax.makeCalcdata = function(trace, coord) {
var arrayIn = trace[coord];
var len = trace._length;
var arrayOut, i;
var _d2c = function(v) { return ax.d2c(v, trace.thetaunit); };
if(arrayIn) {
if(Lib.isTypedArray(arrayIn) && axType === 'linear') {
if(len === arrayIn.length) {
return arrayIn;
} else if(arrayIn.subarray) {
return arrayIn.subarray(0, len);
}
}
arrayOut = new Array(len);
for(i = 0; i < len; i++) {
arrayOut[i] = _d2c(arrayIn[i]);
}
} else {
var coord0 = coord + '0';
var dcoord = 'd' + coord;
var v0 = (coord0 in trace) ? _d2c(trace[coord0]) : 0;
var dv = (trace[dcoord]) ? _d2c(trace[dcoord]) : (ax.period || 2 * Math.PI) / len;
arrayOut = new Array(len);
for(i = 0; i < len; i++) {
arrayOut[i] = v0 + i * dv;
}
}
return arrayOut;
};

Depending on how slow this conversion can be, moving it to the calc step will help ensure faster interactions (note that the calc is skipped on e.g. zoom and pan).

This might be a fairly big job though, you'll have to replace all the isArrayOrTypedArray calls upstream of the calc step with something like isArrayOrTypedArrayOrIsPrimitiveTypedArrayRepr (or something less verbose 😄 ).

So I guess, we should first benchmark primitiveTypedArrayReprToTypedArray what kind of potential perf gain we could get.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@etpinard Maybe I'm misunderstanding something. As it is now, I thought these conversions would be happening in the supplyDefaults logic for the various traces. Does that logic run on events like zoom/pan?

Either way, I'll get some performance numbers for primitiveTypedArrayReprToTypedArray. I was hoping I wouldn't need to retrace all of the steps you went through to add the original TypedArray support!

var coercedV = primitiveTypedArrayReprToTypedArray(v);
if(coercedV === undefined && dflt !== undefined) {
propOut.set(dflt);
} else {
propOut.set(coercedV);
}
} else if(dflt !== undefined) {
propOut.set(dflt);
}
}
},
enumerated: {
Expand Down Expand Up @@ -392,6 +406,10 @@ exports.coerce = function(containerIn, containerOut, attributes, attribute, dflt
if(opts.arrayOk && isArrayOrTypedArray(v)) {
propOut.set(v);
return v;
} else if(opts.arrayOk && isPrimitiveTypedArrayRepr(v)) {
var coercedV = primitiveTypedArrayReprToTypedArray(v);
propOut.set(coercedV);
return coercedV;
}

var coerceFunction = exports.valObjectMeta[opts.valType].coerceFunction;
Expand Down Expand Up @@ -521,3 +539,48 @@ function validate(value, opts) {
return out !== failed;
}
exports.validate = validate;

var dtypeStringToTypedarrayType = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that all of them, except for Uint8ClampedArray:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Typed_arrays#Typed_array_views

Might as well add it here for completeness.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in 5030d3a

int8: Int8Array,
int16: Int16Array,
int32: Int32Array,
uint8: Uint8Array,
uint16: Uint16Array,
uint32: Uint32Array,
float32: Float32Array,
float64: Float64Array
};

/**
* Convert a primitive TypedArray representation object into a TypedArray
* @param {object} v: Object with `dtype` and `data` properties that
* represens a TypedArray.
*
* @returns {TypedArray}
*/
function primitiveTypedArrayReprToTypedArray(v) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious. Have you benchmarked this routine here for bas64 string corresponding to 1e4, 1e5, and 1e6 pts?

// v has dtype and data properties

// Get TypedArray constructor type
var TypeArrayType = dtypeStringToTypedarrayType[v.dtype];

// Process data
var coercedV;
var data = v.data;
if(data instanceof ArrayBuffer) {
// data is an ArrayBuffer
coercedV = new TypeArrayType(data);
} else if(data.constructor === DataView) {
// data has a buffer property, where the buffer is an ArrayBuffer
coercedV = new TypeArrayType(data.buffer);
} else if(Array.isArray(data)) {
// data is a primitive array
coercedV = new TypeArrayType(data);
} else if(typeof data === 'string' ||
data instanceof String) {
// data is a base64 encoded string
var buffer = b64.decode(data);
coercedV = new TypeArrayType(buffer);
}
return coercedV;
}
9 changes: 8 additions & 1 deletion src/lib/is_array.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,15 @@ function isArray1D(a) {
return !isArrayOrTypedArray(a[0]);
}

function isPrimitiveTypedArrayRepr(a) {
return (a !== undefined && a !== null &&
archmoj marked this conversation as resolved.
Show resolved Hide resolved
typeof a === 'object' &&
a.hasOwnProperty('dtype') && a.hasOwnProperty('data'));
}

module.exports = {
isTypedArray: isTypedArray,
isArrayOrTypedArray: isArrayOrTypedArray,
isArray1D: isArray1D
isArray1D: isArray1D,
isPrimitiveTypedArrayRepr: isPrimitiveTypedArrayRepr
};
13 changes: 13 additions & 0 deletions test/image/mocks/typed_array_repr_scatter.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"data": [{
"type": "scatter",
"x": {"dtype": "float64", "data": [3, 2, 1]},
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure about using "data", "data" has a pretty important meaning already for plotly.js. I'd vote for values.

Copy link
Contributor

Choose a reason for hiding this comment

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

or maybe just "v" as calling a base64 string a set of values sounds wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I renamed data to value (singular) in 5030d3a. Does that work for you? v felt too short 🙂

"y": {"dtype": "float32", "data": "AABAQAAAAEAAAIA/"},
"marker": {
"color": {
"dtype": "uint16",
"data": "AwACAAEA",
},
}
}]
}
148 changes: 148 additions & 0 deletions test/jasmine/tests/primitive_typed_array_repr_test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
var Lib = require('@src/lib');
var supplyDefaults = require('../assets/supply_defaults');
var isTypedArray = require('../../../src/lib/is_array').isTypedArray;
var b64 = require('base64-arraybuffer');
var mock1 = require('@mocks/typed_array_repr_scatter.json');

var typedArraySpecs = [
['int8', new Int8Array([-128, -34, 1, 127])],
['uint8', new Uint8Array([0, 1, 127, 255])],
['int16', new Int16Array([-32768, -123, 345, 32767])],
['uint16', new Uint16Array([0, 345, 32767, 65535])],
['int32', new Int32Array([-2147483648, -123, 345, 32767, 2147483647])],
['uint32', new Uint32Array([0, 345, 32767, 4294967295])],
['float32', new Float32Array([1.2E-38, -2345.25, 2.7182818, 3.1415926, 2, 3.4E38])],
['float64', new Float64Array([5.0E-324, 2.718281828459045, 3.141592653589793, 1.8E308])]
];

describe('Test TypedArray representations', function() {
'use strict';

describe('ArrayBuffer', function() {
it('should accept representation as ArrayBuffer', function() {
typedArraySpecs.forEach(function(arraySpec) {
// Build data and confirm its type
var data = arraySpec[1].buffer;
expect(data.constructor).toEqual(ArrayBuffer);

var repr = {
dtype: arraySpec[0],
data: data
};
var gd = {
data: [{
y: repr
}],
};

supplyDefaults(gd);

expect(gd.data[0].y).toEqual(repr);
expect(gd._fullData[0].y).toEqual(arraySpec[1]);
});
});
});

describe('Array', function() {
it('should accept representation as Array', function() {
typedArraySpecs.forEach(function(arraySpec) {
// Build data and confirm its type
var data = Array.prototype.slice.call(arraySpec[1]);
expect(Array.isArray(data)).toEqual(true);

var repr = {
dtype: arraySpec[0],
data: data
};
var gd = {
data: [{
y: repr
}],
};

supplyDefaults(gd);

expect(gd.data[0].y).toEqual(repr);
expect(gd._fullData[0].y).toEqual(arraySpec[1]);
});
});
});

describe('DataView', function() {
it('should accept representation as DataView', function() {
typedArraySpecs.forEach(function(arraySpec) {
// Build data and confirm its type
var data = new DataView(arraySpec[1].buffer);
expect(data.constructor).toEqual(DataView);

var repr = {
dtype: arraySpec[0],
data: data
};
var gd = {
data: [{
y: repr
}],
};

supplyDefaults(gd);

expect(gd.data[0].y).toEqual(repr);
expect(gd._fullData[0].y).toEqual(arraySpec[1]);
});
});
});

describe('base64', function() {
it('should accept representation as base 64 string', function() {
typedArraySpecs.forEach(function(arraySpec) {
// Build data and confirm its type
var data = b64.encode(arraySpec[1].buffer);
expect(typeof data).toEqual('string');

var repr = {
dtype: arraySpec[0],
data: data
};
var gd = {
data: [{
y: repr
}],
};

supplyDefaults(gd);
expect(gd.data[0].y).toEqual(repr);
expect(gd._fullData[0].y).toEqual(arraySpec[1]);
});
});
});

describe('mock', function() {
it('should accept representation as base 64 and Array in Mock', function() {

var gd = Lib.extendDeep({}, mock1);
supplyDefaults(gd);

// Check x
// data_array property
expect(gd.data[0].x).toEqual({
'dtype': 'float64',
'data': [3, 2, 1]});
expect(gd._fullData[0].x).toEqual(new Float64Array([3, 2, 1]));

// Check y
// data_array property
expect(gd.data[0].y).toEqual({
'dtype': 'float32',
'data': 'AABAQAAAAEAAAIA/'});
expect(gd._fullData[0].y).toEqual(new Float32Array([3, 2, 1]));

// Check marker.color
// This is an arrayOk property not a data_array property
expect(gd.data[0].marker.color).toEqual({
'dtype': 'uint16',
'data': 'AwACAAEA'});
expect(gd._fullData[0].marker.color).toEqual(new Uint16Array([3, 2, 1]));
});
});
});