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

Porting to JavaScript #812

Merged
merged 4 commits into from Apr 11, 2017

Conversation

Projects
None yet
3 participants
@Semigradsky
Member

Semigradsky commented Apr 8, 2017

#810

⚠️ Not ready for merge yet.

After third commit performance was ruined. Now it's more than 20x times slower.

I will investigate and update PR.

@ai

This comment has been minimized.

Show comment
Hide comment
@ai

ai Apr 8, 2017

Member

Can you send it again to v7 branch?

Member

ai commented Apr 8, 2017

Can you send it again to v7 branch?

@Semigradsky Semigradsky changed the base branch from master to v7 Apr 9, 2017

@Semigradsky

This comment has been minimized.

Show comment
Hide comment
@Semigradsky

Semigradsky Apr 10, 2017

Member

Speed has been restored. On my PC:

> @ test D:\github\benchmark
> gulp "prefixers"

[13:01:36] Using gulpfile D:\github\benchmark\gulpfile.js
[13:01:36] Starting 'bootstrap'...
[13:01:36] Finished 'bootstrap' after 331 μs
[13:01:36] Starting 'prefixers'...
[13:01:37] Running suite Bootstrap [D:\github\benchmark\prefixers.js]...
[13:01:43]    Autoprefixer dev x 15.36 ops/sec ±11.01% (51 runs sampled)
[13:01:49]    Autoprefixer x 16.07 ops/sec ±10.66% (53 runs sampled)
[13:01:49] Fastest test is Autoprefixer at 1.05x faster than Autoprefixer dev

Autoprefixer:     62 ms
Autoprefixer dev: 65 ms (1.0 times slower)
Member

Semigradsky commented Apr 10, 2017

Speed has been restored. On my PC:

> @ test D:\github\benchmark
> gulp "prefixers"

[13:01:36] Using gulpfile D:\github\benchmark\gulpfile.js
[13:01:36] Starting 'bootstrap'...
[13:01:36] Finished 'bootstrap' after 331 μs
[13:01:36] Starting 'prefixers'...
[13:01:37] Running suite Bootstrap [D:\github\benchmark\prefixers.js]...
[13:01:43]    Autoprefixer dev x 15.36 ops/sec ±11.01% (51 runs sampled)
[13:01:49]    Autoprefixer x 16.07 ops/sec ±10.66% (53 runs sampled)
[13:01:49] Fastest test is Autoprefixer at 1.05x faster than Autoprefixer dev

Autoprefixer:     62 ms
Autoprefixer dev: 65 ms (1.0 times slower)
@ai

This comment has been minimized.

Show comment
Hide comment
@ai

ai Apr 10, 2017

Member

Great! What else do we need to do before merge?

Member

ai commented Apr 10, 2017

Great! What else do we need to do before merge?

@Semigradsky

This comment has been minimized.

Show comment
Hide comment
@Semigradsky

Semigradsky Apr 10, 2017

Member

I think it's ready to merge.

Member

Semigradsky commented Apr 10, 2017

I think it's ready to merge.

@lydell

This comment has been minimized.

Show comment
Hide comment
@lydell

lydell Apr 10, 2017

Contributor

@Semigradsky Did you find why the 20x slowdown happened? I'm super curious! :)

Contributor

lydell commented Apr 10, 2017

@Semigradsky Did you find why the 20x slowdown happened? I'm super curious! :)

@ai

This comment has been minimized.

Show comment
Hide comment
@ai

ai Apr 10, 2017

Member

OK, I will try to look on this week

Member

ai commented Apr 10, 2017

OK, I will try to look on this week

@Semigradsky

This comment has been minimized.

Show comment
Hide comment
@Semigradsky

Semigradsky Apr 10, 2017

Member

@lydell this line:
f67569e#diff-1ea033eede7961e81fac67f7cf52fd51R316

At first I changed this line to if (node._autoprefixerDisabled) {.
It ruined performance.

Member

Semigradsky commented Apr 10, 2017

@lydell this line:
f67569e#diff-1ea033eede7961e81fac67f7cf52fd51R316

At first I changed this line to if (node._autoprefixerDisabled) {.
It ruined performance.

@ai ai merged commit b206662 into postcss:v7 Apr 11, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ai

This comment has been minimized.

Show comment
Hide comment
@ai

ai Apr 11, 2017

Member

You forget to fix data/ and test/ :'(

Member

ai commented Apr 11, 2017

You forget to fix data/ and test/ :'(

ai added a commit that referenced this pull request Apr 13, 2017

Porting to JavaScript (#812)
* Rename files

This commit needed for saving git file history

* Run decaffeinate and autofix eslint errors

* Make it work

* Manual editing

ai added a commit that referenced this pull request May 1, 2017

Porting to JavaScript (#812)
* Rename files

This commit needed for saving git file history

* Run decaffeinate and autofix eslint errors

* Make it work

* Manual editing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment