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

Tried to speed up tokenizer using Uint32Array #1163

Closed
wants to merge 17 commits into from

Conversation

@lily-moon
Copy link

@lily-moon lily-moon commented Jul 1, 2018

I've changed tokenizer and parser code according to #1162. Tokenizer uses one instance of Uint32Array.
To make this work I also needed to fix tests.

As I see there is no change in perfomance. Moreover using one instance of Uint32Array in tokenizer leads to creating new instances in parser to fill array of tokens or brackets.

And as a result, GC starts less frequently in tokenizer and more frequently in parser.

@@ -79,43 +95,83 @@ export default function tokenizer (input, options = {}) {
code === CR ||
code === FEED
)

currentToken = ['space', css.slice(pos, next)]
currentToken = new Uint32Array([tokenCodes.SPACE, pos, next])

This comment has been minimized.

@vitkarpov

vitkarpov Jul 1, 2018
Contributor

I believe Uint32Array could be an implementation detail behind object of Token type, proposed some refactoring first.

As you actually made this happen, what can you tell about readability/maintainability of this code?

@vitkarpov
Copy link
Contributor

@vitkarpov vitkarpov commented Jul 1, 2018

As I see there is no change in perfomance

Bummer :(

@@ -22,33 +22,36 @@ export default class Parser {
this.tokenizer = tokenizer(this.input)
}

getTokenContent (token) {
return this.input.css.slice(token[1], token[2])

This comment has been minimized.

@hzlmn

hzlmn Jul 1, 2018
Contributor

Wondering, is it good for performance? Raw css string could be massive and slicing it each time not a good idea as for me.

This comment has been minimized.

@lily-moon

lily-moon Jul 1, 2018
Author

Yeah, I agree.
In the task, here; https://cultofmartians.com/tasks/postcss-tokenizer.html was mentioned that parser may take content as a raw css substring, that is why I did that.
If the main goal is to check whether tokenizer works faster with Uint32Array, this part of parcer's code may be appropriate temporary for this check.

@vitkarpov
Copy link
Contributor

@vitkarpov vitkarpov commented Jul 1, 2018

@lily-moon btw, can you share perf tests results?

@lily-moon
Copy link
Author

@lily-moon lily-moon commented Jul 1, 2018

Test was runned on node v8.11.3, Windows 10, Intel Core i7-8550U, 8 GB RAM

PostCSS x 77.54 ops/sec ±5.67% (45 runs sampled)
PostCSS dev x 38.67 ops/sec ±2.38% (56 runs sampled)
Fastest test is PostCSS at 2.0x faster than PostCSS dev

PostCSS:     13 ms
PostCSS dev: 26 ms (2.0 times slower)
@vitkarpov
Copy link
Contributor

@vitkarpov vitkarpov commented Jul 1, 2018

So it's become worse? 🤔

@ai
Copy link
Member

@ai ai commented Jul 1, 2018

@lily-moon Wow, it was really fast 👍 great work

It is very strange, that after these changes performance was even reduced. I thought that at least it will be in the same level. Can I ask you to run Node.js profiler?

@ai
Copy link
Member

@ai ai commented Jul 1, 2018

@lily-moon I found potential leak of performance:

currentToken.set([tokenCodes.SPACE, pos, next])

When you use currentToken.set you pass an array here. So you create an array every time when you want to fill a currentToken. As result we have many objects again and GC will be executed.

Can you try to replace currentToken.set([]) to direct access without creating array currentToken[0] = value?

@ai ai force-pushed the postcss:amy branch from 36fc78a to 76b169b Jul 1, 2018
@ai
Copy link
Member

@ai ai commented Jul 1, 2018

function fillToken (...args) {

Will create a array again (to put it in args)

@lily-moon
Copy link
Author

@lily-moon lily-moon commented Jul 1, 2018

Now perf test shows following:

PostCSS x 134 ops/sec ±5.08% (69 runs sampled)
PostCSS dev x 118 ops/sec ±4.59% (83 runs sampled)
Fastest test is PostCSS at 1.14x faster than PostCSS dev
PostCSS:     7 ms
PostCSS dev: 8 ms (1.1 times slower)

Ok! I'll avoid args

@ai
Copy link
Member

@ai ai commented Jul 1, 2018

@lily-moon cool, waiting for results =^_^=

@lily-moon
Copy link
Author

@lily-moon lily-moon commented Jul 1, 2018

perf test result is following:

PostCSS x 67.18 ops/sec ±4.58% (39 runs sampled)
PostCSS dev x 54.98 ops/sec ±5.92% (61 runs sampled)
Fastest test is PostCSS at 1.22x faster than PostCSS dev

PostCSS:     15 ms
PostCSS dev: 18 ms (1.2 times slower)
@ai
Copy link
Member

@ai ai commented Jul 1, 2018

Hm. Is it tokenizer benchmark? What about gulp parsers results?

@lily-moon
Copy link
Author

@lily-moon lily-moon commented Jul 1, 2018

[22:09:53]    Rework x 25.90 ops/sec ±9.69% (47 runs sampled)
[22:09:59]    PostCSS dev x 28.71 ops/sec ±8.25% (62 runs sampled)
[22:10:05]    PostCSS x 55.20 ops/sec ±8.91% (55 runs sampled)
[22:10:12]    PostCSS Full x 14.46 ops/sec ±11.18% (39 runs sampled)
[22:10:17]    CSSOM x 66.08 ops/sec ±11.74% (61 runs sampled)
[22:10:23]    Mensch x 32.95 ops/sec ±10.08% (53 runs sampled)
[22:10:29]    Gonzales x 8.33 ops/sec ±19.45% (27 runs sampled)
[22:10:36]    Gonzales PE x 8.88 ops/sec ±17.51% (27 runs sampled)
[22:10:41]    CSSTree x 155 ops/sec ±7.64% (68 runs sampled)
[22:10:47]    ParserLib x 5.96 ops/sec ±16.64% (19 runs sampled)
[22:10:53]    Stylecow x 20.02 ops/sec ±12.80% (37 runs sampled)
[22:10:58]    Stylis x 90.92 ops/sec ±10.15% (62 runs sampled)
[22:10:59] Fastest test is CSSTree at 1.71x faster than Stylis

CSSTree:      6 ms   (2.8 times faster)
Stylis:       11 ms  (1.6 times faster)
CSSOM:        15 ms  (1.2 times faster)
PostCSS:      18 ms
Mensch:       30 ms  (1.7 times slower)
PostCSS dev:  35 ms  (1.9 times slower)
Rework:       39 ms  (2.1 times slower)
Stylecow:     50 ms  (2.8 times slower)
PostCSS Full: 69 ms  (3.8 times slower)
Gonzales PE:  113 ms (6.2 times slower)
Gonzales:     120 ms (6.6 times slower)
ParserLib:    168 ms (9.3 times slower)
@ai
Copy link
Member

@ai ai commented Jul 1, 2018

Sad. OK, the last idea for optimization:

  1. Let’s remove clearToken
  2. Let’s write directly to currentToken instead of calling some function (function call is not cheap)
@ai
Copy link
Member

@ai ai commented Jul 1, 2018

I think we can remove currentToken.fill(0) completely

@lily-moon
Copy link
Author

@lily-moon lily-moon commented Jul 1, 2018

Done.

gulp tokenizers result:

[22:31:54]    PostCSS x 71.49 ops/sec ±5.81% (35 runs sampled)
[22:32:00]    PostCSS dev x 73.00 ops/sec ±3.93% (46 runs sampled)
[22:32:00] Fastest test is PostCSS dev at 1.02x faster than PostCSS

PostCSS dev: 14 ms (1.0 times faster)
PostCSS:     14 ms

gulp parsers result:

[22:32:11]    Rework x 26.28 ops/sec ±10.14% (47 runs sampled)
[22:32:17]    PostCSS dev x 33.38 ops/sec ±11.70% (46 runs sampled)
[22:32:23]    PostCSS x 49.60 ops/sec ±11.25% (52 runs sampled)
[22:32:29]    PostCSS Full x 11.79 ops/sec ±8.48% (52 runs sampled)
[22:32:35]    CSSOM x 61.15 ops/sec ±9.92% (55 runs sampled)
[22:32:40]    Mensch x 32.44 ops/sec ±11.65% (45 runs sampled)
[22:32:47]    Gonzales x 7.95 ops/sec ±18.52% (26 runs sampled)
[22:32:54]    Gonzales PE x 6.86 ops/sec ±21.86% (24 runs sampled)
[22:33:00]    CSSTree x 96.59 ops/sec ±13.78% (52 runs sampled)
[22:33:07]    ParserLib x 5.59 ops/sec ±18.80% (18 runs sampled)
[22:33:13]    Stylecow x 16.89 ops/sec ±16.36% (33 runs sampled)
[22:33:18]    Stylis x 83.96 ops/sec ±11.00% (57 runs sampled)
[22:33:18] Fastest tests are CSSTree,Stylis at 1.15x faster than Stylis

CSSTree:      10 ms  (1.9 times faster)
Stylis:       12 ms  (1.7 times faster)
CSSOM:        16 ms  (1.2 times faster)
PostCSS:      20 ms
PostCSS dev:  30 ms  (1.5 times slower)
Mensch:       31 ms  (1.5 times slower)
Rework:       38 ms  (1.9 times slower)
Stylecow:     59 ms  (2.9 times slower)
PostCSS Full: 85 ms  (4.2 times slower)
Gonzales:     126 ms (6.2 times slower)
Gonzales PE:  146 ms (7.2 times slower)
ParserLib:    179 ms (8.9 times slower)

Thats's without currentToken.fill(0) deleting

@lily-moon
Copy link
Author

@lily-moon lily-moon commented Jul 1, 2018

gulp tokenizers result:

[22:53:38]    PostCSS x 67.46 ops/sec ±3.18% (56 runs sampled)
[22:53:45]    PostCSS dev x 73.06 ops/sec ±2.45% (67 runs sampled)
[22:53:45] Fastest test is PostCSS dev at 1.08x faster than PostCSS

PostCSS dev: 14 ms (1.1 times faster)
PostCSS:     15 ms

gulp parsers result:

[22:54:04]    Rework x 25.91 ops/sec ±10.13% (48 runs sampled)
[22:54:10]    PostCSS dev x 43.83 ops/sec ±9.30% (55 runs sampled)
[22:54:17]    PostCSS x 58.40 ops/sec ±4.51% (66 runs sampled)
[22:54:23]    PostCSS Full x 14.43 ops/sec ±10.96% (39 runs sampled)
[22:54:29]    CSSOM x 61.17 ops/sec ±11.69% (57 runs sampled)
[22:54:35]    Mensch x 32.21 ops/sec ±16.25% (51 runs sampled)
[22:54:42]    Gonzales x 8.48 ops/sec ±17.16% (28 runs sampled)
[22:54:48]    Gonzales PE x 9.11 ops/sec ±16.88% (26 runs sampled)
[22:54:53]    CSSTree x 103 ops/sec ±20.06% (63 runs sampled)
[22:55:01]    ParserLib x 5.66 ops/sec ±16.54% (20 runs sampled)
[22:55:06]    Stylecow x 20.58 ops/sec ±13.34% (38 runs sampled)
[22:55:12]    Stylis x 86.71 ops/sec ±9.97% (61 runs sampled)
[22:55:12] Fastest test is CSSTree at 1.19x faster than Stylis

CSSTree:      10 ms  (1.8 times faster)
Stylis:       12 ms  (1.5 times faster)
CSSOM:        16 ms  (1.0 times faster)
PostCSS:      17 ms
PostCSS dev:  23 ms  (1.3 times slower)
Mensch:       31 ms  (1.8 times slower)
Rework:       39 ms  (2.3 times slower)
Stylecow:     49 ms  (2.8 times slower)
PostCSS Full: 69 ms  (4.0 times slower)
Gonzales PE:  110 ms (6.4 times slower)
Gonzales:     118 ms (6.9 times slower)
ParserLib:    177 ms (10.3 times slower)

And Jest tests of tokenizer and parser fail, I did not edited them.

@ai
Copy link
Member

@ai ai commented Jul 1, 2018

OK, thanks for trying the idea. It is OK to have no success in optimization task since it is always making 10 attempts with only 1 success. You do a great work very quickly.

I will keep this PR open for a day, maybe somebody will suggest an additional idea to improve the result.

@lily-moon
Copy link
Author

@lily-moon lily-moon commented Jul 1, 2018

And thank you for your time! It was a usefull experience for me!

@ai ai closed this Jul 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.