Skip to content

Fix date parsing#269

Merged
puzrin merged 2 commits intonodeca:masterfrom
dplepage:fix-date-parsing
Mar 16, 2016
Merged

Fix date parsing#269
puzrin merged 2 commits intonodeca:masterfrom
dplepage:fix-date-parsing

Conversation

@dplepage
Copy link
Copy Markdown
Contributor

Fixes #268.

@puzrin
Copy link
Copy Markdown
Member

puzrin commented Mar 16, 2016

Did you checked benchmark? I understand problem, but don't like idea to increase number of regex calls.

@dplepage
Copy link
Copy Markdown
Contributor Author

The benchmark results look pretty similar on my machine, but this is by no means exhaustive. It doesn't look like any of the benchmark samples actually contain timestamps, so this is really only checking the slowdown of resolveYamlTimestamp. The differences appear to be within the variance I see from just running the benchmark multiple times.

Before

Sample: document_application2.yaml (360 characters)
 > current x 40,145 ops/sec ±1.55% (81 runs sampled)
Sample: document_huge.yaml (7182063 characters)
 > current x 4.99 ops/sec ±3.08% (15 runs sampled)
Sample: document_nodeca_application.yaml (2055 characters)
 > current x 33,652 ops/sec ±1.11% (89 runs sampled)
Sample: document_nodeca_database.yaml (394 characters)
 > current x 73,385 ops/sec ±1.56% (85 runs sampled)
Sample: document_nodeca_logger.yaml (727 characters)
 > current x 25,125 ops/sec ±1.51% (83 runs sampled)
Sample: scalar_double_quoted_binary_01.yaml (807 characters)
 > current x 129,090 ops/sec ±0.92% (86 runs sampled)
Sample: scalar_double_quoted_binary_02.yaml (12007 characters)
 > current x 8,955 ops/sec ±0.89% (83 runs sampled)
Sample: scalar_double_quoted_escaped_01.yaml (119 characters)
 > current x 522,485 ops/sec ±1.32% (85 runs sampled)
Sample: scalar_double_quoted_escaped_02.yaml (3444 characters)
 > current x 35,455 ops/sec ±1.58% (84 runs sampled)
Sample: scalar_double_quoted_simple_01.yaml (107 characters)
 > current x 774,084 ops/sec ±1.21% (86 runs sampled)
Sample: scalar_double_quoted_simple_02.yaml (3353 characters)
 > current x 40,661 ops/sec ±1.27% (87 runs sampled)
Sample: scalar_double_quoted_simple_multiline.yaml (2707 characters)
 > current x 37,276 ops/sec ±1.09% (85 runs sampled)

After

Sample: document_application2.yaml (360 characters)
> current x 41,155 ops/sec ±1.75% (81 runs sampled)
Sample: document_huge.yaml (7182063 characters)
> current x 5.09 ops/sec ±2.67% (15 runs sampled)
Sample: document_nodeca_application.yaml (2055 characters)
> current x 32,444 ops/sec ±0.77% (88 runs sampled)
Sample: document_nodeca_database.yaml (394 characters)
> current x 83,429 ops/sec ±1.52% (84 runs sampled)
Sample: document_nodeca_logger.yaml (727 characters)
> current x 24,824 ops/sec ±0.93% (88 runs sampled)
Sample: scalar_double_quoted_binary_01.yaml (807 characters)
> current x 121,229 ops/sec ±0.75% (86 runs sampled)
Sample: scalar_double_quoted_binary_02.yaml (12007 characters)
> current x 9,258 ops/sec ±0.82% (88 runs sampled)
Sample: scalar_double_quoted_escaped_01.yaml (119 characters)
> current x 560,702 ops/sec ±1.09% (86 runs sampled)
Sample: scalar_double_quoted_escaped_02.yaml (3444 characters)
> current x 25,252 ops/sec ±1.34% (88 runs sampled)
Sample: scalar_double_quoted_simple_01.yaml (107 characters)
> current x 772,908 ops/sec ±0.96% (88 runs sampled)
Sample: scalar_double_quoted_simple_02.yaml (3353 characters)
> current x 40,198 ops/sec ±0.96% (88 runs sampled)
Sample: scalar_double_quoted_simple_multiline.yaml (2707 characters)
> current x 42,134 ops/sec ±0.99% (90 runs sampled)

@puzrin
Copy link
Copy Markdown
Member

puzrin commented Mar 16, 2016

Ок, if benchmarks did not suffered - i see no problems.

/cc @dervus ?

@dervus
Copy link
Copy Markdown
Collaborator

dervus commented Mar 16, 2016

LGFM.

puzrin pushed a commit that referenced this pull request Mar 16, 2016
@puzrin puzrin merged commit a5981c0 into nodeca:master Mar 16, 2016
@dplepage dplepage deleted the fix-date-parsing branch March 16, 2016 20:50
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.

3 participants