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

Feature console crate feature #800

Merged
merged 3 commits into from
Oct 6, 2020
Merged

Feature console crate feature #800

merged 3 commits into from
Oct 6, 2020

Conversation

HalidOdat
Copy link
Member

This Pull Request fixes/closes #718 .

It changes the following:

  • Put console object behind a feature flag
  • Documented features

@HalidOdat HalidOdat added enhancement New feature or request builtins PRs and Issues related to builtins/intrinsics API labels Oct 5, 2020
@HalidOdat HalidOdat added this to the v0.11.0 milestone Oct 5, 2020
@codecov
Copy link

codecov bot commented Oct 5, 2020

Codecov Report

Merging #800 into master will increase coverage by 0.46%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #800      +/-   ##
==========================================
+ Coverage   59.15%   59.61%   +0.46%     
==========================================
  Files         155      155              
  Lines        9841     9965     +124     
==========================================
+ Hits         5821     5941     +120     
- Misses       4020     4024       +4     
Impacted Files Coverage Δ
boa/src/builtins/console/mod.rs 23.03% <ø> (+3.93%) ⬆️
boa/src/builtins/mod.rs 25.00% <0.00%> (-0.81%) ⬇️
boa/src/context.rs 70.67% <ø> (+0.53%) ⬆️
boa/src/lib.rs 86.36% <ø> (ø)
boa/src/value/mod.rs 72.10% <ø> (-0.19%) ⬇️
boa/src/syntax/lexer/identifier.rs 62.50% <0.00%> (-22.12%) ⬇️
boa/src/syntax/parser/function/mod.rs 67.69% <0.00%> (-3.74%) ⬇️
boa/src/syntax/parser/statement/mod.rs 54.08% <0.00%> (-2.35%) ⬇️
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d6c252d...938e2c9. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Oct 5, 2020

Benchmark for 32e0d1f

Click to view benchmark
Test PR Benchmark Master Benchmark %
Arithmetic operations (Execution) 379.9±31.04ns 378.1±27.69ns +0.48%
Arithmetic operations (Full) 250.7±18.19µs 272.6±21.23µs -8.03%
Array access (Execution) 8.5±0.69µs 8.7±0.69µs -2.30%
Array access (Full) 283.5±22.55µs 310.5±26.19µs -8.70%
Array creation (Execution) 3.5±0.29ms 3.3±0.21ms +6.06%
Array creation (Full) 3.8±0.31ms 3.9±0.28ms -2.56%
Array pop (Execution) 1213.5±75.20µs 1200.7±79.77µs +1.07%
Array pop (Full) 1713.1±108.01µs 1790.8±124.69µs -4.34%
Boolean Object Access (Execution) 5.1±0.39µs 5.4±0.43µs -5.56%
Boolean Object Access (Full) 273.8±20.61µs 294.9±25.77µs -7.15%
Clean js (Execution) 790.7±58.38µs 774.6±75.70µs +2.08%
Clean js (Full) 1112.4±97.27µs 1104.8±85.59µs +0.69%
Clean js (Parser) 38.2±2.48µs 38.0±2.46µs +0.53%
Create Realm 485.1±33.37ns 471.2±30.47ns +2.95%
Dynamic Object Property Access (Execution) 6.0±0.34µs 6.4±0.48µs -6.25%
Dynamic Object Property Access (Full) 276.6±20.83µs 297.2±21.63µs -6.93%
Expression (Parser) 7.3±0.50µs 7.5±0.54µs -2.67%
Fibonacci (Execution) 927.6±59.99µs 942.2±65.01µs -1.55%
Fibonacci (Full) 1197.6±81.99µs 1280.0±96.99µs -6.44%
For loop (Execution) 23.7±1.49µs 25.0±2.19µs -5.20%
For loop (Full) 286.0±20.05µs 304.4±16.18µs -6.04%
For loop (Parser) 18.7±1.34µs 18.8±1.47µs -0.53%
Goal Symbols (Parser) 12.6±0.93µs 12.6±1.03µs 0.00%
Hello World (Parser) 3.3±0.28µs 3.3±0.21µs 0.00%
Long file (Parser) 817.5±72.05ns 819.4±62.58ns -0.23%
Mini js (Execution) 705.2±56.57µs 692.4±48.81µs +1.85%
Mini js (Full) 991.0±88.63µs 1009.5±72.90µs -1.83%
Mini js (Parser) 33.8±2.50µs 34.4±2.43µs -1.74%
Number Object Access (Execution) 3.9±0.23µs 4.2±0.20µs -7.14%
Number Object Access (Full) 267.6±22.30µs 295.1±21.40µs -9.32%
Object Creation (Execution) 5.1±0.41µs 5.1±0.41µs 0.00%
Object Creation (Full) 290.0±31.39µs 290.2±19.55µs -0.07%
RegExp (Execution) 71.7±5.34µs 71.7±6.37µs 0.00%
RegExp (Full) 374.5±28.72µs 379.1±27.23µs -1.21%
RegExp Literal (Execution) 77.4±6.52µs 77.0±4.96µs +0.52%
RegExp Literal (Full) 353.0±19.90µs 386.9±28.13µs -8.76%
RegExp Literal Creation (Execution) 72.6±5.86µs 73.6±6.22µs -1.36%
RegExp Literal Creation (Full) 355.9±23.85µs 386.7±35.57µs -7.96%
Static Object Property Access (Execution) 5.3±0.36µs 5.5±0.38µs -3.64%
Static Object Property Access (Full) 270.3±14.29µs 289.9±18.37µs -6.76%
String Object Access (Execution) 7.5±0.68µs 7.6±0.52µs -1.32%
String Object Access (Full) 267.1±17.54µs 291.3±18.27µs -8.31%
String comparison (Execution) 6.9±0.57µs 6.8±0.39µs +1.47%
String comparison (Full) 271.8±14.21µs 293.3±21.39µs -7.33%
String concatenation (Execution) 5.5±0.38µs 5.8±0.37µs -5.17%
String concatenation (Full) 269.6±16.58µs 283.5±18.45µs -4.90%
String copy (Execution) 4.3±0.34µs 4.3±0.34µs 0.00%
String copy (Full) 259.2±22.79µs 278.3±18.02µs -6.86%
Symbols (Execution) 3.5±0.25µs 3.6±0.18µs -2.78%
Symbols (Full) 242.2±18.07µs 275.9±20.86µs -12.21%

boa/src/lib.rs Outdated Show resolved Hide resolved
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.

Just the 2 typos, the rest looks fine.

@github-actions
Copy link

github-actions bot commented Oct 6, 2020

Benchmark for 31b71b4

Click to view benchmark
Test PR Benchmark Master Benchmark %
Arithmetic operations (Execution) 361.9±8.97ns 347.7±17.51ns +4.08%
Arithmetic operations (Full) 238.0±9.38µs 244.4±7.79µs -2.62%
Array access (Execution) 8.2±0.42µs 7.7±0.29µs +6.49%
Array access (Full) 281.9±9.58µs 282.9±10.29µs -0.35%
Array creation (Execution) 3.2±0.19ms 3.0±0.11ms +6.67%
Array creation (Full) 3.5±0.08ms 3.4±0.20ms +2.94%
Array pop (Execution) 1101.0±42.62µs 1040.5±52.06µs +5.81%
Array pop (Full) 1622.3±64.29µs 1582.1±106.12µs +2.54%
Boolean Object Access (Execution) 5.0±0.10µs 4.9±0.18µs +2.04%
Boolean Object Access (Full) 249.1±11.08µs 257.8±11.54µs -3.37%
Clean js (Execution) 726.7±18.37µs 714.6±38.17µs +1.69%
Clean js (Full) 1012.7±47.78µs 982.4±35.29µs +3.08%
Clean js (Parser) 36.7±2.31µs 34.9±2.00µs +5.16%
Create Realm 451.4±22.26ns 447.8±32.42ns +0.80%
Dynamic Object Property Access (Execution) 5.6±0.80µs 5.6±0.18µs 0.00%
Dynamic Object Property Access (Full) 264.0±14.22µs 269.4±10.55µs -2.00%
Expression (Parser) 6.5±0.30µs 6.2±0.24µs +4.84%
Fibonacci (Execution) 856.4±30.70µs 871.4±36.92µs -1.72%
Fibonacci (Full) 1181.3±41.11µs 1168.3±51.00µs +1.11%
For loop (Execution) 23.1±1.08µs 22.2±0.86µs +4.05%
For loop (Full) 276.2±10.47µs 303.8±26.48µs -9.08%
For loop (Parser) 18.6±1.86µs 16.3±0.54µs +14.11%
Goal Symbols (Parser) 12.2±0.61µs 11.3±0.49µs +7.96%
Hello World (Parser) 3.2±0.26µs 2.9±0.13µs +10.34%
Long file (Parser) 852.9±144.28ns 738.7±37.73ns +15.46%
Mini js (Execution) 652.5±17.40µs 647.0±34.78µs +0.85%
Mini js (Full) 919.2±44.59µs 896.7±35.93µs +2.51%
Mini js (Parser) 30.8±1.25µs 30.3±1.07µs +1.65%
Number Object Access (Execution) 3.9±0.13µs 3.9±0.16µs 0.00%
Number Object Access (Full) 244.6±17.19µs 255.9±6.80µs -4.42%
Object Creation (Execution) 4.9±0.28µs 4.7±0.19µs +4.26%
Object Creation (Full) 256.7±7.10µs 274.0±12.97µs -6.31%
RegExp (Execution) 68.4±4.75µs 66.2±3.21µs +3.32%
RegExp (Full) 331.8±13.94µs 339.3±13.09µs -2.21%
RegExp Literal (Execution) 74.2±3.61µs 70.6±3.72µs +5.10%
RegExp Literal (Full) 331.7±16.45µs 343.1±17.24µs -3.32%
RegExp Literal Creation (Execution) 67.8±3.42µs 67.2±2.79µs +0.89%
RegExp Literal Creation (Full) 359.9±53.37µs 342.4±27.28µs +5.11%
Static Object Property Access (Execution) 4.9±0.22µs 4.9±0.20µs 0.00%
Static Object Property Access (Full) 251.2±13.64µs 273.9±11.72µs -8.29%
String Object Access (Execution) 7.2±0.35µs 7.1±0.37µs +1.41%
String Object Access (Full) 251.2±13.79µs 283.3±22.30µs -11.33%
String comparison (Execution) 6.6±0.15µs 6.3±0.28µs +4.76%
String comparison (Full) 260.3±17.66µs 277.3±13.55µs -6.13%
String concatenation (Execution) 5.6±0.37µs 5.3±0.36µs +5.66%
String concatenation (Full) 256.1±12.65µs 256.2±8.66µs -0.04%
String copy (Execution) 4.1±0.17µs 4.0±0.18µs +2.50%
String copy (Full) 245.7±9.27µs 262.1±9.47µs -6.26%
Symbols (Execution) 3.4±0.21µs 3.3±0.15µs +3.03%
Symbols (Full) 234.2±7.38µs 258.9±10.20µs -9.54%

@Lan2u Lan2u merged commit 39b4ead into master Oct 6, 2020
@HalidOdat HalidOdat deleted the feature/console-feature branch October 6, 2020 11:13
@croraf
Copy link
Contributor

croraf commented Oct 6, 2020

I don't think this is the correct way to go. Console should be injected into the context (during context creation or dynamically) from boa-cli or any other user of boa. It should not be a part of boa so the feature flag should apply on the boa-cli.

@HalidOdat
Copy link
Member Author

HalidOdat commented Oct 6, 2020

I don't think this is the correct way to go. Console should be injected into the context (during context creation or dynamically) from boa-cli or any other user of boa. It should not be a part of boa so the feature flag should apply on the boa-cli.

The console feature is off by default, if the user whats a quick default implementation of the console spec that prints as string they can enable it, but if the user whats a more advanced console object like firefox/chrome console object where it is interactive and you can click things (not just a string) they can define their own implementation.

@Razican
Copy link
Member

Razican commented Oct 6, 2020

I don't think this is the correct way to go. Console should be injected into the context (during context creation or dynamically) from boa-cli or any other user of boa. It should not be a part of boa so the feature flag should apply on the boa-cli.

The console feature is off by default, if the user whats a quick default implementation of the console spec that prints as string they can enable it, but if the user whats a more advanced console object like firefox/chrome console object where it is interactive and you can click things (not just a string) they can define their own implementation.

Should this be a separate crate?

@croraf
Copy link
Contributor

croraf commented Oct 6, 2020

@HalidOdat If the user wants a quick default implementation of console which he doesnt have, he should use the console crate we aslo provide and initialize the boa instance providing this crate. This is negligible more work comparing to enabling a feature.

And boa-cli will do that by default. (so the feature can be on the boa-cli, enabled by default)

@HalidOdat
Copy link
Member Author

HalidOdat commented Oct 6, 2020

Should this be a separate crate?

@HalidOdat If the user wants a quick default implementation of console which he doesnt have, he should use the console crate we aslo provide and initialize the boa instance providing this crate. This is negligible more work comparing to enabling a feature.

And boa-cli will do that by default. (so the feature can be on the boa-cli, enabled by default)

Yes. This is probably the best choice, Im reopening the issue :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API builtins PRs and Issues related to builtins/intrinsics enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Split WebAPI (environment) built-in objects from the ES builtin objects.
5 participants