Skip to content
This repository has been archived by the owner on Dec 5, 2022. It is now read-only.

Update Buffer to use Node v8 syntax #154

Merged
merged 4 commits into from
Jun 6, 2017
Merged

Update Buffer to use Node v8 syntax #154

merged 4 commits into from
Jun 6, 2017

Conversation

addityasingh
Copy link
Contributor

@vigneshshanmugam
Copy link
Collaborator

vigneshshanmugam commented Jun 6, 2017

Can you run the benchmark with Buffer.from and Buffer.allocUnsafe?

Also zero fill the buffer with Buffer.allowUnsafe(0)

@addityasingh
Copy link
Contributor Author

The benchmark tests are failing for me with a TCP socket timeout error

@vigneshshanmugam
Copy link
Collaborator

vigneshshanmugam commented Jun 6, 2017

Tested the benchmarks with

With buffer.from

Tailor Overhead Metrics {
  "type": "histogram",
  "min": 0.16044899999997142,
  "max": 51.542193999999995,
  "sum": 4812.023433999978,
  "variance": 1.5396951250332829,
  "mean": 0.4806736024373219,
  "std_dev": 1.240844520894251,
  "count": 10011,
  "median": 0.3414210000000253,
  "p75": 0.44491949999996905,
  "p95": 0.7591170000000087,
  "p99": 1.922573709999998,
  "p999": 50.62678896100011
}

With allocUnsafe and fill

Tailor Overhead Metrics {
  "type": "histogram",
  "min": 0.16987999999997783,
  "max": 66.15484600000002,
  "sum": 5323.567117000002,
  "variance": 2.0253315784060777,
  "mean": 0.5344410317237258,
  "std_dev": 1.4231414470832047,
  "count": 9961,
  "median": 0.38589300000001003,
  "p75": 0.5101299999999895,
  "p95": 0.9364355499999815,
  "p99": 3.4285971000000406,
  "p999": 19.733977497999973
}


@vigneshshanmugam vigneshshanmugam merged commit b7f3b06 into zalando:master Jun 6, 2017
@addityasingh
Copy link
Contributor Author

looks like Buffer.alloc() numbers look better than allocUnsafe() and fill() upto p99. 👍

@addityasingh
Copy link
Contributor Author

@vigneshshanmugam Can you check and confirm if the examples with parse-template are working for you after this change?

node examples/fragment-performance
node examples/multiple-fragments-with-custom-amd

The above 2 seems to be broken with error:

TypeError: this.html.charCodeAt is not a function
at Preprocessor.advance (/Users/adisingh/Documents/dev/projects/node/tailor/node_modules/parse5/lib/tokenizer/preprocessor.js:124:24)
at Tokenizer._consume (/Users/adisingh/Documents/dev/projects/node/tailor/node_modules/parse5/lib/tokenizer/index.js:278:30)
at Tokenizer.getNextToken (/Users/adisingh/Documents/dev/projects/node/tailor/node_modules/parse5/lib/tokenizer/index.js:236:23)
at Parser._runParsingLoop (/Users/adisingh/Documents/dev/projects/node/tailor/node_modules/parse5/lib/parser/index.js:393:36)
at Parser.parse (/Users/adisingh/Documents/dev/projects/node/tailor/node_modules/parse5/lib/parser/index.js:322:10)
at Object.parse (/Users/adisingh/Documents/dev/projects/node/tailor/node_modules/parse5/lib/index.js:11:19)
at Transform.applyTransforms (/Users/adisingh/Documents/dev/projects/node/tailor/lib/transform.js:27:34)
at Promise.resolve.then.transform (/Users/adisingh/Documents/dev/projects/node/tailor/lib/parse-template.js:14:43)

@vigneshshanmugam
Copy link
Collaborator

Weird. It works properly for me and the tests are passing on all versions.

Can you give the node version?

@addityasingh
Copy link
Contributor Author

node v7.8.0

rikkert added a commit to rikkert/tailor that referenced this pull request Jun 14, 2017
* upstream/master:
  change the hooks logic for fragment
  Fix hooks
  Add unit test for fetch template.js (zalando#161)
  fix saucelabs key
  Update Buffer to use Node v8 syntax (zalando#154)
  Group the tests by features, refactor tailor instance creation i… (zalando#152)
  fix integration test and remove duplication (zalando#151)
  Provide options to parse multiple Header Links: maxAssetsLinks (zalando#140)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix deprecation warnings in Node 8
2 participants