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

Added "js" feature for getrandom for WebAssembly builds #1521

Merged
merged 4 commits into from
Sep 19, 2021
Merged

Conversation

Razican
Copy link
Member

@Razican Razican commented Aug 27, 2021

This Pull Request fixes/closes #1475.

It changes the following:

  • When building for wasm-pack, this adds the "js" feature to the getrandom dependency, to be able to build it.
  • Updated dependencies
  • Fixed a small clippy error

Interestingly, our build check didn't see this issue. Maybe we should check if it's a cache issue.

We are also getting this warning we should somehow fix (but I don't know how):

WARNING in ./boa_wasm/pkg/index_bg.js 195:14-60
Critical dependency: the request of a dependency is an expression
    at CommonJsRequireContextDependency.getWarnings (./node_modules/webpack/lib/dependencies/ContextDependency.js:82:18)
    at Compilation.reportDependencyErrorsAndWarnings (./node_modules/webpack/lib/Compilation.js:2586:24)
    at ./node_modules/webpack/lib/Compilation.js:2204:10
    at eval (eval at create (./node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:29:1)
    at ./node_modules/webpack/lib/FlagDependencyExportsPlugin.js:376:11
    at ./node_modules/neo-async/async.js:2830:7
    at Object.each (./node_modules/neo-async/async.js:2850:39)
    at ./node_modules/webpack/lib/FlagDependencyExportsPlugin.js:355:18
    at ./node_modules/neo-async/async.js:2830:7
    at Object.each (./node_modules/neo-async/async.js:2850:39)
 @ ./boa_wasm/pkg/index.js 2:0-30 2:0-30
 @ ./index.js 4:13-37

webpack 5.51.1 compiled with 1 warning in 13037 ms

@Razican Razican added bug Something isn't working webassembly Anything related to using Boa with WASM labels Aug 27, 2021
@Razican Razican added this to the v0.13.0 milestone Aug 27, 2021
@github-actions
Copy link

github-actions bot commented Aug 27, 2021

Test262 conformance changes:

Test result master count PR count difference
Total 80,804 80,918 +114
Passed 33,174 33,178 +4
Ignored 15,896 15,898 +2
Failed 31,734 31,842 +108
Panics 0 0 0
Conformance 41.05% 41.00% -0.05%
Fixed tests (2):
test/built-ins/Date/prototype/valueOf/S9.4_A3_T2.js [strict mode] (previously Failed)
test/built-ins/Date/prototype/valueOf/S9.4_A3_T2.js (previously Failed)

@github-actions
Copy link

Benchmark for f9daa93

Click to view benchmark
Test PR Benchmark Master Benchmark %
Arithmetic operations (Execution) 261.4±8.29ns 270.7±9.35ns -3.44%
Arithmetic operations (Full) 333.1±12.99µs 334.8±10.03µs -0.51%
Array access (Execution) 7.6±0.21µs 7.9±0.26µs -3.80%
Array access (Full) 372.9±21.09µs 375.5±14.19µs -0.69%
Array creation (Execution) 3.0±0.07ms 2.9±0.09ms +3.45%
Array creation (Full) 3.3±0.07ms 3.4±0.07ms -2.94%
Array pop (Execution) 974.3±40.30µs 961.2±33.49µs +1.36%
Array pop (Full) 1498.2±47.05µs 1538.9±36.70µs -2.64%
Boolean Object Access (Execution) 5.8±0.24µs 5.8±0.21µs 0.00%
Boolean Object Access (Full) 358.7±11.92µs 354.5±7.96µs +1.18%
Clean js (Execution) 801.2±20.75µs 791.8±22.07µs +1.19%
Clean js (Full) 1191.2±46.49µs 1194.3±33.03µs -0.26%
Clean js (Parser) 37.8±1.82µs 37.5±1.18µs +0.80%
Create Realm 429.9±17.86ns 441.8±14.39ns -2.69%
Dynamic Object Property Access (Execution) 5.9±0.19µs 6.0±0.20µs -1.67%
Dynamic Object Property Access (Full) 364.3±10.50µs 360.0±10.05µs +1.19%
Expression (Parser) 6.3±0.19µs 6.3±0.23µs 0.00%
Fibonacci (Execution) 926.3±39.09µs 939.4±22.37µs -1.39%
Fibonacci (Full) 1305.1±41.17µs 1324.4±76.87µs -1.46%
For loop (Execution) 21.9±0.55µs 22.6±1.82µs -3.10%
For loop (Full) 371.3±18.97µs 375.5±18.13µs -1.12%
For loop (Parser) 17.9±0.42µs 17.7±0.91µs +1.13%
Goal Symbols (Parser) 13.1±0.39µs 13.0±0.51µs +0.77%
Hello World (Parser) 3.7±0.15µs 3.7±0.21µs 0.00%
Long file (Parser) 815.8±21.94ns 820.8±28.79ns -0.61%
Mini js (Execution) 737.1±32.53µs 717.1±33.91µs +2.79%
Mini js (Full) 1115.2±34.53µs 1120.8±40.61µs -0.50%
Mini js (Parser) 33.6±1.88µs 33.1±1.04µs +1.51%
Number Object Access (Execution) 4.6±0.26µs 4.5±0.16µs +2.22%
Number Object Access (Full) 353.6±12.97µs 353.3±17.34µs +0.08%
Object Creation (Execution) 5.2±0.19µs 5.4±0.18µs -3.70%
Object Creation (Full) 359.2±11.79µs 361.2±18.91µs -0.55%
RegExp (Execution) 14.2±0.37µs 14.8±0.56µs -4.05%
RegExp (Full) 369.0±10.00µs 371.1±15.10µs -0.57%
RegExp Literal (Execution) 14.3±0.61µs 15.1±2.33µs -5.30%
RegExp Literal (Full) 380.4±13.38µs 377.3±25.00µs +0.82%
RegExp Literal Creation (Execution) 10.2±0.40µs 10.4±0.32µs -1.92%
RegExp Literal Creation (Full) 369.0±13.97µs 360.7±9.31µs +2.30%
Static Object Property Access (Execution) 5.4±0.15µs 5.6±0.19µs -3.57%
Static Object Property Access (Full) 359.1±10.58µs 360.2±9.48µs -0.31%
String Object Access (Execution) 8.3±0.30µs 8.3±0.24µs 0.00%
String Object Access (Full) 363.9±22.83µs 362.8±13.93µs +0.30%
String comparison (Execution) 7.4±0.28µs 7.5±0.23µs -1.33%
String comparison (Full) 362.4±14.27µs 359.7±12.59µs +0.75%
String concatenation (Execution) 5.9±0.25µs 6.0±0.15µs -1.67%
String concatenation (Full) 356.2±9.60µs 352.6±12.18µs +1.02%
String copy (Execution) 4.6±0.13µs 4.8±0.11µs -4.17%
String copy (Full) 351.8±11.42µs 346.4±9.58µs +1.56%
Symbols (Execution) 3.9±0.11µs 4.0±0.19µs -2.50%
Symbols (Full) 338.0±15.12µs 333.1±10.33µs +1.47%

Copy link
Contributor

@RageKnify RageKnify left a comment

Choose a reason for hiding this comment

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

LGTM, minor nitpick

boa/src/builtins/array/tests.rs Outdated Show resolved Hide resolved
Copy link
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Member

@HalidOdat HalidOdat left a comment

Choose a reason for hiding this comment

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

Looks good just needs a rebase! :)

@Razican Razican merged commit 9a915c3 into master Sep 19, 2021
@Razican Razican deleted the fix_yarn_build branch September 19, 2021 19:06
bors bot pushed a commit that referenced this pull request Feb 20, 2022
This Pull Request fixes/closes #1670.

It changes the following:

- Removes the "js" feature by default from getrandom for wasm (still there for boa_wasm)
- Updates dependencies

Note that this change was introduced in #1521, after #1475. We must make sure that the issue doesn't come back.
Razican added a commit that referenced this pull request Feb 21, 2022
This Pull Request fixes/closes #1670.

It changes the following:

- Removes the "js" feature by default from getrandom for wasm (still there for boa_wasm)
- Updates dependencies

Note that this change was introduced in #1521, after #1475. We must make sure that the issue doesn't come back.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working webassembly Anything related to using Boa with WASM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error: Cannot find module 'webpack-cli/bin/config-yargs'
4 participants