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

Adding support for TypedArrays #640

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

calumrussell
Copy link

@calumrussell calumrussell commented Aug 15, 2023

I have added support for Uint8Array with a test showing that it works as expected. No additional types have been created, this works by simply treating a Uint8Array like a normal array. Afaik, you cannot create a nested array this way so the changes are quite limited.

Support for more types of TypedArray is easy to add but as I worked on this issue further I realised that there may be wider implications for this:

  • Does this conflict with JSONSchema?
  • TypedArrays appear to be strongly typed (for example, creating a Uint8Array with values longer than 8 bits will always be cast down to 8 bit values) so would it be possible to improve perf by removing type checks (I looked at this briefly but I didn't see how it could be done without broader changes).
  • Which TypedArray should be supported?

Related to - #626 - I haven't verified the performance difference either, I am assuming this can only come from removing type checks?

Checklist

index.js Outdated
@@ -586,7 +590,7 @@ function buildArray (context, location) {
`

functionCode += `
if (!Array.isArray(obj)) {
if (!Array.isArray(obj) && !(obj != null && (${supportedTypedArrays.map(type => ' obj.constructor.name === \'' + type + '\' ').join('||')}) )) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ugh. It would be better to put the array in a "global" scope and make an indexOf by the obj.constructor.name than this imho.

I think this will slow down array significantly.

Copy link
Author

Choose a reason for hiding this comment

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

I am not familiar with perf implications here.

Would Array.includes also be worth testing?

@Uzlopak Uzlopak added the benchmark Label to run benchmark against PR and main branch label Aug 15, 2023
Copy link
Member

@ivan-tymoshenko ivan-tymoshenko left a comment

Choose a reason for hiding this comment

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

This would not work in places where we use Ajv. To test it put an array under anyOf, oneOf or if.

@github-actions
Copy link

PR:

short string............................................. x 13,595,841 ops/sec ±1.51% (184 runs sampled)
short string with double quote........................... x 9,215,276 ops/sec ±1.55% (183 runs sampled)
long string without double quotes........................ x 25,779 ops/sec ±1.50% (182 runs sampled)
long string.............................................. x 10,345 ops/sec ±1.46% (186 runs sampled)
number................................................... x 896,130,427 ops/sec ±1.17% (184 runs sampled)
integer.................................................. x 198,751,726 ops/sec ±1.43% (181 runs sampled)
formatted date-time...................................... x 2,369,994 ops/sec ±1.32% (184 runs sampled)
formatted date........................................... x 1,656,788 ops/sec ±1.38% (183 runs sampled)
formatted time........................................... x 1,638,795 ops/sec ±1.31% (184 runs sampled)
short array of numbers................................... x 53,333 ops/sec ±1.53% (178 runs sampled)
short array of integers.................................. x 47,163 ops/sec ±1.81% (184 runs sampled)
short array of short strings............................. x 13,496 ops/sec ±1.20% (181 runs sampled)
short array of long strings.............................. x 13,338 ops/sec ±1.33% (182 runs sampled)
short array of objects with properties of different types x 6,793 ops/sec ±1.56% (182 runs sampled)
object with number property.............................. x 896,784,352 ops/sec ±1.24% (185 runs sampled)
object with integer property............................. x 193,037,691 ops/sec ±1.14% (184 runs sampled)
object with short string property........................ x 13,774,587 ops/sec ±1.27% (182 runs sampled)
object with long string property......................... x 10,398 ops/sec ±1.32% (184 runs sampled)
object with properties of different types................ x 1,403,202 ops/sec ±1.60% (179 runs sampled)
simple object............................................ x 7,110,621 ops/sec ±1.33% (185 runs sampled)
simple object with required fields....................... x 7,041,745 ops/sec ±1.37% (183 runs sampled)
object with const string property........................ x 875,952,643 ops/sec ±1.30% (182 runs sampled)
object with const number property........................ x 897,199,115 ops/sec ±1.33% (186 runs sampled)
object with const bool property.......................... x 904,853,523 ops/sec ±1.23% (184 runs sampled)
object with const object property........................ x 886,073,478 ops/sec ±1.32% (182 runs sampled)
object with const null property.......................... x 902,604,508 ops/sec ±1.35% (183 runs sampled)

MASTER:

short string............................................. x 14,150,071 ops/sec ±1.28% (185 runs sampled)
short string with double quote........................... x 9,779,016 ops/sec ±1.25% (184 runs sampled)
long string without double quotes........................ x 26,082 ops/sec ±1.46% (180 runs sampled)
long string.............................................. x 10,256 ops/sec ±1.43% (182 runs sampled)
number................................................... x 892,098,738 ops/sec ±1.08% (184 runs sampled)
integer.................................................. x 202,345,214 ops/sec ±1.16% (187 runs sampled)
formatted date-time...................................... x 2,430,098 ops/sec ±0.89% (185 runs sampled)
formatted date........................................... x 1,664,159 ops/sec ±1.09% (185 runs sampled)
formatted time........................................... x 1,654,243 ops/sec ±1.28% (184 runs sampled)
short array of numbers................................... x 55,645 ops/sec ±1.60% (184 runs sampled)
short array of integers.................................. x 48,082 ops/sec ±1.40% (181 runs sampled)
short array of short strings............................. x 13,352 ops/sec ±1.41% (181 runs sampled)
short array of long strings.............................. x 12,081 ops/sec ±0.97% (181 runs sampled)
short array of objects with properties of different types x 6,302 ops/sec ±1.36% (181 runs sampled)
object with number property.............................. x 805,934,162 ops/sec ±1.33% (182 runs sampled)
object with integer property............................. x 189,319,369 ops/sec ±1.34% (185 runs sampled)
object with short string property........................ x 13,993,875 ops/sec ±1.24% (186 runs sampled)
object with long string property......................... x 10,526 ops/sec ±1.15% (185 runs sampled)
object with properties of different types................ x 1,390,259 ops/sec ±1.73% (181 runs sampled)
simple object............................................ x 7,115,977 ops/sec ±1.22% (184 runs sampled)
simple object with required fields....................... x 7,092,319 ops/sec ±1.42% (180 runs sampled)
object with const string property........................ x 887,052,086 ops/sec ±1.38% (181 runs sampled)
object with const number property........................ x 859,910,878 ops/sec ±1.50% (179 runs sampled)
object with const bool property.......................... x 863,841,739 ops/sec ±1.46% (181 runs sampled)
object with const object property........................ x 855,027,252 ops/sec ±1.42% (182 runs sampled)
object with const null property.......................... x 896,106,854 ops/sec ±1.22% (184 runs sampled)

@github-actions github-actions bot removed the benchmark Label to run benchmark against PR and main branch label Aug 15, 2023
@calumrussell
Copy link
Author

This would not work in places where we use Ajv. To test it put an array under anyOf, oneOf or if.

It seems like there is no way to solve this?

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.

None yet

3 participants