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

Define all property methods of constructors #1109

Merged
merged 2 commits into from
Feb 6, 2021

Conversation

RageKnify
Copy link
Member

This Pull Request will help work on #400.

It changes the following:

  • rename property method in ConstructorBuilder to data_property
  • rename static_property method in ConstructorBuilder to static_data_property
  • introduce accessor variants similar to above functions
  • introduce property_descriptor variants to offer more generic approach

@RageKnify RageKnify added builtins PRs and Issues related to builtins/intrinsics API labels Feb 2, 2021
@github-actions
Copy link

github-actions bot commented Feb 2, 2021

Test262 conformance changes:

Test result master count PR count difference
Total 78,497 78,497 0
Passed 24,986 24,985 -1
Ignored 15,587 15,587 0
Failed 37,924 37,925 +1
Panics 16 17 +1
Conformance 31.83 31.83 -0.00%

@codecov
Copy link

codecov bot commented Feb 2, 2021

Codecov Report

Merging #1109 (be9e641) into master (9b1264f) will decrease coverage by 0.11%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1109      +/-   ##
==========================================
- Coverage   58.64%   58.53%   -0.12%     
==========================================
  Files         176      176              
  Lines       12523    12547      +24     
==========================================
  Hits         7344     7344              
- Misses       5179     5203      +24     
Impacted Files Coverage Δ
boa/src/class.rs 0.00% <0.00%> (ø)
boa/src/object/mod.rs 49.47% <0.00%> (-1.90%) ⬇️

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 9b1264f...7315bf2. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Feb 2, 2021

Benchmark for 4137948

Click to view benchmark
Test PR Benchmark Master Benchmark %
Arithmetic operations (Execution) 361.8±0.74ns 359.8±0.97ns +0.56%
Arithmetic operations (Full) 231.4±0.64µs 234.1±0.57µs -1.15%
Array access (Execution) 6.4±0.03µs 6.4±0.04µs 0.00%
Array access (Full) 252.6±0.56µs 253.8±1.17µs -0.47%
Array creation (Execution) 3.0±0.01ms 3.0±0.04ms 0.00%
Array creation (Full) 3.2±0.01ms 3.2±0.01ms 0.00%
Array pop (Execution) 966.3±2.97µs 968.5±3.00µs -0.23%
Array pop (Full) 1396.0±1.95µs 1393.0±3.65µs +0.22%
Boolean Object Access (Execution) 5.3±0.01µs 5.2±0.02µs +1.92%
Boolean Object Access (Full) 250.3±0.64µs 251.5±1.72µs -0.48%
Clean js (Execution) 681.6±3.68µs 676.0±4.35µs +0.83%
Clean js (Full) 973.1±5.17µs 971.1±6.71µs +0.21%
Clean js (Parser) 40.3±0.20µs 40.3±0.18µs 0.00%
Create Realm 452.9±0.35ns 450.6±4.82ns +0.51%
Dynamic Object Property Access (Execution) 5.2±0.02µs 5.1±0.04µs +1.96%
Dynamic Object Property Access (Full) 250.3±0.31µs 253.7±2.00µs -1.34%
Expression (Parser) 6.9±0.02µs 6.9±0.01µs 0.00%
Fibonacci (Execution) 851.2±0.54µs 848.1±1.04µs +0.37%
Fibonacci (Full) 1120.3±1.85µs 1115.2±5.88µs +0.46%
For loop (Execution) 23.0±0.10µs 23.0±0.09µs 0.00%
For loop (Full) 274.1±0.60µs 272.4±1.30µs +0.62%
For loop (Parser) 19.3±0.04µs 19.1±0.04µs +1.05%
Goal Symbols (Parser) 13.6±0.04µs 13.6±0.08µs 0.00%
Hello World (Parser) 3.6±0.01µs 3.6±0.02µs 0.00%
Long file (Parser) 796.0±1.16ns 791.2±3.25ns +0.61%
Mini js (Execution) 610.1±3.07µs 606.5±4.17µs +0.59%
Mini js (Full) 882.3±4.23µs 878.4±3.56µs +0.44%
Mini js (Parser) 35.8±0.95µs 35.6±0.25µs +0.56%
Number Object Access (Execution) 4.1±0.01µs 4.0±0.02µs +2.50%
Number Object Access (Full) 245.6±0.95µs 244.2±1.44µs +0.57%
Object Creation (Execution) 4.3±0.01µs 4.4±0.02µs -2.27%
Object Creation (Full) 245.6±0.26µs 249.8±1.25µs -1.68%
RegExp (Execution) 9.6±0.03µs 9.6±0.05µs 0.00%
RegExp (Full) 253.6±0.84µs 255.1±0.86µs -0.59%
RegExp Literal (Execution) 11.1±0.03µs 11.0±0.07µs +0.91%
RegExp Literal (Full) 257.3±0.62µs 258.3±2.57µs -0.39%
RegExp Literal Creation (Execution) 9.6±0.03µs 9.6±0.07µs 0.00%
RegExp Literal Creation (Full) 249.2±0.35µs 250.1±0.73µs -0.36%
Static Object Property Access (Execution) 4.6±0.02µs 4.7±0.03µs -2.13%
Static Object Property Access (Full) 248.5±0.44µs 248.6±0.59µs -0.04%
String Object Access (Execution) 7.2±0.02µs 7.2±0.03µs 0.00%
String Object Access (Full) 253.3±0.53µs 253.6±0.48µs -0.12%
String comparison (Execution) 6.6±0.02µs 6.6±0.03µs 0.00%
String comparison (Full) 252.7±1.01µs 251.8±0.64µs +0.36%
String concatenation (Execution) 5.3±0.03µs 5.3±0.02µs 0.00%
String concatenation (Full) 243.1±0.42µs 246.9±0.63µs -1.54%
String copy (Execution) 4.0±0.01µs 3.9±0.03µs +2.56%
String copy (Full) 241.8±0.61µs 240.6±1.59µs +0.50%
Symbols (Execution) 3.4±0.02µs 3.4±0.01µs 0.00%
Symbols (Full) 235.0±0.47µs 234.4±1.08µs +0.26%

RageKnify added a commit that referenced this pull request Feb 3, 2021
Includes change to Value::hash so that 1i32 == 1f64

closes #400, blocked by #1109
@RageKnify RageKnify added this to the v0.12.0 milestone Feb 3, 2021
Copy link
Contributor

@tofpie tofpie 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 to me!

@RageKnify
Copy link
Member Author

RageKnify commented Feb 4, 2021

I'm gonna try to figure out which panic was introduced and fixed it, feel free to review, just don't merge yet.
panic disappeared

6 = 3 * 2
2 instance and static properties
3 data, accessor and properties

discussed in #400
@github-actions
Copy link

github-actions bot commented Feb 4, 2021

Benchmark for f013ec9

Click to view benchmark
Test PR Benchmark Master Benchmark %
Arithmetic operations (Execution) 360.8±4.40ns 364.3±1.05ns -0.96%
Arithmetic operations (Full) 235.1±0.62µs 238.0±1.49µs -1.22%
Array access (Execution) 6.3±0.09µs 6.5±0.04µs -3.08%
Array access (Full) 247.8±5.02µs 256.8±1.13µs -3.50%
Array creation (Execution) 2.9±0.03ms 2.9±0.01ms 0.00%
Array creation (Full) 3.2±0.07ms 3.3±0.01ms -3.03%
Array pop (Execution) 928.2±15.50µs 944.3±4.89µs -1.70%
Array pop (Full) 1424.9±10.96µs 1432.1±2.79µs -0.50%
Boolean Object Access (Execution) 5.0±0.11µs 5.2±0.03µs -3.85%
Boolean Object Access (Full) 252.4±0.43µs 251.5±0.96µs +0.36%
Clean js (Execution) 665.6±12.12µs 680.8±4.86µs -2.23%
Clean js (Full) 985.2±6.73µs 982.2±12.31µs +0.31%
Clean js (Parser) 40.5±0.09µs 39.7±0.62µs +2.02%
Create Realm 452.9±4.34ns 460.1±1.19ns -1.56%
Dynamic Object Property Access (Execution) 5.0±0.07µs 5.1±0.03µs -1.96%
Dynamic Object Property Access (Full) 253.4±0.64µs 250.3±3.45µs +1.24%
Expression (Parser) 6.9±0.02µs 6.8±0.05µs +1.47%
Fibonacci (Execution) 834.4±8.63µs 851.0±3.07µs -1.95%
Fibonacci (Full) 1117.2±7.88µs 1134.1±18.71µs -1.49%
For loop (Execution) 22.5±0.23µs 22.8±0.08µs -1.32%
For loop (Full) 272.9±4.18µs 279.2±0.74µs -2.26%
For loop (Parser) 19.2±0.03µs 19.1±0.24µs +0.52%
Goal Symbols (Parser) 13.7±0.05µs 13.5±0.12µs +1.48%
Hello World (Parser) 3.6±0.03µs 3.6±0.03µs 0.00%
Long file (Parser) 813.3±1.41ns 810.2±9.98ns +0.38%
Mini js (Execution) 599.6±9.51µs 606.5±3.83µs -1.14%
Mini js (Full) 897.5±3.71µs 893.0±4.46µs +0.50%
Mini js (Parser) 35.6±0.12µs 35.2±0.49µs +1.14%
Number Object Access (Execution) 4.1±0.02µs 4.0±0.02µs +2.50%
Number Object Access (Full) 249.5±0.39µs 246.1±0.68µs +1.38%
Object Creation (Execution) 4.3±0.05µs 4.4±0.02µs -2.27%
Object Creation (Full) 250.0±3.60µs 246.4±2.57µs +1.46%
RegExp (Execution) 9.4±0.14µs 9.5±0.05µs -1.05%
RegExp (Full) 259.3±1.38µs 252.2±2.67µs +2.82%
RegExp Literal (Execution) 10.7±0.17µs 10.8±0.10µs -0.93%
RegExp Literal (Full) 260.2±0.57µs 257.6±2.85µs +1.01%
RegExp Literal Creation (Execution) 9.2±0.27µs 9.4±0.05µs -2.13%
RegExp Literal Creation (Full) 255.7±0.56µs 250.7±3.23µs +1.99%
Static Object Property Access (Execution) 4.5±0.07µs 4.6±0.02µs -2.17%
Static Object Property Access (Full) 249.1±1.76µs 248.3±3.20µs +0.32%
String Object Access (Execution) 7.0±0.11µs 7.0±0.04µs 0.00%
String Object Access (Full) 255.9±0.81µs 254.8±0.44µs +0.43%
String comparison (Execution) 6.6±0.06µs 6.6±0.07µs 0.00%
String comparison (Full) 255.2±1.82µs 253.9±2.49µs +0.51%
String concatenation (Execution) 5.4±0.02µs 5.3±0.04µs +1.89%
String concatenation (Full) 246.6±0.59µs 244.1±2.12µs +1.02%
String copy (Execution) 4.0±0.04µs 4.0±0.03µs 0.00%
String copy (Full) 240.5±1.05µs 242.1±1.98µs -0.66%
Symbols (Execution) 3.3±0.03µs 3.4±0.01µs -2.94%
Symbols (Full) 235.0±4.17µs 236.2±1.68µs -0.51%

boa/src/object/mod.rs Outdated Show resolved Hide resolved
boa/src/object/mod.rs Outdated Show resolved Hide resolved
boa/src/object/mod.rs Outdated Show resolved Hide resolved
As per review in #1109 by @HalidOdat

Co-authored-by: Halid Odat <halidodat@gmail.com>
@github-actions
Copy link

github-actions bot commented Feb 5, 2021

Benchmark for be9e641

Click to view benchmark
Test PR Benchmark Master Benchmark %
Arithmetic operations (Execution) 365.2±12.31ns 365.5±18.44ns -0.08%
Arithmetic operations (Full) 253.1±9.07µs 262.9±13.92µs -3.73%
Array access (Execution) 6.8±0.83µs 7.1±0.38µs -4.23%
Array access (Full) 282.3±19.74µs 277.4±10.25µs +1.77%
Array creation (Execution) 2.8±0.08ms 2.9±0.16ms -3.45%
Array creation (Full) 3.1±0.14ms 3.1±0.11ms 0.00%
Array pop (Execution) 907.6±60.72µs 938.5±24.11µs -3.29%
Array pop (Full) 1393.5±40.63µs 1439.6±54.10µs -3.20%
Boolean Object Access (Execution) 5.5±0.21µs 5.4±0.31µs +1.85%
Boolean Object Access (Full) 271.6±11.25µs 272.2±9.71µs -0.22%
Clean js (Execution) 714.8±31.00µs 691.5±26.84µs +3.37%
Clean js (Full) 995.1±59.18µs 1031.4±46.02µs -3.52%
Clean js (Parser) 44.5±2.10µs 44.0±1.24µs +1.14%
Create Realm 443.4±15.86ns 464.0±22.81ns -4.44%
Dynamic Object Property Access (Execution) 5.8±0.27µs 5.6±0.21µs +3.57%
Dynamic Object Property Access (Full) 276.0±11.47µs 277.7±18.42µs -0.61%
Expression (Parser) 7.3±0.32µs 7.0±0.37µs +4.29%
Fibonacci (Execution) 943.0±39.16µs 956.8±36.28µs -1.44%
Fibonacci (Full) 1238.2±136.89µs 1214.4±50.85µs +1.96%
For loop (Execution) 24.3±1.36µs 25.4±3.36µs -4.33%
For loop (Full) 297.5±6.65µs 294.2±10.82µs +1.12%
For loop (Parser) 20.7±0.83µs 20.9±1.11µs -0.96%
Goal Symbols (Parser) 14.4±0.53µs 14.2±0.44µs +1.41%
Hello World (Parser) 3.9±0.17µs 3.9±0.14µs 0.00%
Long file (Parser) 803.1±27.43ns 803.9±42.90ns -0.10%
Mini js (Execution) 641.2±41.37µs 624.3±20.51µs +2.71%
Mini js (Full) 925.4±25.23µs 939.2±43.44µs -1.47%
Mini js (Parser) 39.1±1.30µs 38.7±1.48µs +1.03%
Number Object Access (Execution) 4.3±0.21µs 4.3±0.22µs 0.00%
Number Object Access (Full) 262.0±10.05µs 270.1±18.47µs -3.00%
Object Creation (Execution) 4.7±0.22µs 4.8±0.25µs -2.08%
Object Creation (Full) 272.9±13.81µs 265.8±19.46µs +2.67%
RegExp (Execution) 10.4±0.40µs 10.6±0.52µs -1.89%
RegExp (Full) 279.6±14.85µs 281.8±11.51µs -0.78%
RegExp Literal (Execution) 11.8±0.39µs 11.9±0.32µs -0.84%
RegExp Literal (Full) 292.4±15.57µs 285.8±16.56µs +2.31%
RegExp Literal Creation (Execution) 10.2±0.44µs 10.3±0.28µs -0.97%
RegExp Literal Creation (Full) 279.6±11.89µs 272.9±9.18µs +2.46%
Static Object Property Access (Execution) 5.0±0.18µs 5.2±0.24µs -3.85%
Static Object Property Access (Full) 275.1±10.64µs 265.9±8.36µs +3.46%
String Object Access (Execution) 7.6±0.35µs 7.6±0.25µs 0.00%
String Object Access (Full) 282.3±14.54µs 273.0±11.31µs +3.41%
String comparison (Execution) 7.1±0.41µs 7.1±0.32µs 0.00%
String comparison (Full) 277.8±16.85µs 287.0±9.84µs -3.21%
String concatenation (Execution) 5.8±0.26µs 5.8±0.24µs 0.00%
String concatenation (Full) 268.4±9.63µs 265.5±13.31µs +1.09%
String copy (Execution) 4.3±0.14µs 4.3±0.20µs 0.00%
String copy (Full) 256.4±9.99µs 264.9±15.48µs -3.21%
Symbols (Execution) 3.8±0.19µs 3.9±0.16µs -2.56%
Symbols (Full) 262.4±12.61µs 260.2±11.59µs +0.85%

@RageKnify RageKnify merged commit a5bc7dd into master Feb 6, 2021
@RageKnify RageKnify deleted the refactor/all_properties branch February 6, 2021 01:04
RageKnify added a commit that referenced this pull request Feb 6, 2021
Includes change to Value::hash so that 1i32 == 1f64

closes #400, blocked by #1109
RageKnify added a commit that referenced this pull request Feb 16, 2021
Includes change to Value::hash so that 1i32 == 1f64

closes #400, blocked by #1109
RageKnify added a commit that referenced this pull request Feb 16, 2021
Includes change to Value::hash so that 1i32 == 1f64

closes #400, blocked by #1109
Razican pushed a commit that referenced this pull request May 22, 2021
* Refactor: Define all property methods of constructors

* Refactor: Simplify naming of ConstructorBuilder methods

As per review in #1109 by @HalidOdat

Co-authored-by: Halid Odat <halidodat@gmail.com>
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants