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

Made String.prototype.lastIndexOf() spec compliant #598

Merged
merged 1 commit into from Aug 1, 2020

Conversation

HalidOdat
Copy link
Member

It changes the following:

  • Made String.prototype.lastIndexOf() spec complaint
  • Added tests

@HalidOdat HalidOdat added bug Something isn't working performance Performance related changes and issues builtins PRs and Issues related to builtins/intrinsics execution Issues or PRs related to code execution labels Jul 30, 2020
@HalidOdat HalidOdat added this to the v0.10.0 milestone Jul 30, 2020
@codecov
Copy link

codecov bot commented Jul 30, 2020

Codecov Report

Merging #598 into master will increase coverage by 0.24%.
The diff coverage is 97.40%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #598      +/-   ##
==========================================
+ Coverage   71.26%   71.51%   +0.24%     
==========================================
  Files         177      177              
  Lines       11598    11650      +52     
==========================================
+ Hits         8265     8331      +66     
+ Misses       3333     3319      -14     
Impacted Files Coverage Δ
boa/src/builtins/string/mod.rs 50.54% <87.50%> (+3.67%) ⬆️
boa/src/builtins/string/tests.rs 100.00% <100.00%> (ø)

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 9fdadf8...6ca1354. Read the comment docs.

@github-actions
Copy link

Benchmark for 10c5635

Click to view benchmark
Test PR Benchmark Master Benchmark %
Arithmetic operations (Execution) 350.5±8.86ns 363.6±11.74ns -3.60%
Arithmetic operations (Full) 175.2±5.20µs 174.4±3.81µs +0.46%
Array access (Execution) 8.8±0.32µs 8.8±0.35µs 0.00%
Array access (Full) 192.7±5.10µs 196.1±6.09µs -1.73%
Array creation (Execution) 3.1±0.07ms 3.1±0.11ms 0.00%
Array creation (Full) 3.5±0.11ms 3.4±0.12ms +2.94%
Array pop (Execution) 1184.6±67.11µs 1164.3±78.17µs +1.74%
Array pop (Full) 1481.4±53.50µs 1429.7±30.72µs +3.62%
Boolean Object Access (Execution) 4.6±0.12µs 4.5±0.13µs +2.22%
Boolean Object Access (Full) 186.9±4.58µs 186.5±5.42µs +0.21%
Create Realm 433.1±12.98ns 426.9±12.61ns +1.45%
Dynamic Object Property Access (Execution) 5.5±0.24µs 5.5±0.23µs 0.00%
Dynamic Object Property Access (Full) 196.0±10.44µs 190.9±5.66µs +2.67%
Expression (Lexer) 2.3±0.07µs 2.4±0.18µs -4.17%
Expression (Parser) 5.1±0.14µs 5.1±0.19µs 0.00%
Fibonacci (Execution) 1023.4±37.01µs 1031.4±55.41µs -0.78%
Fibonacci (Full) 1217.2±41.25µs 1237.8±41.81µs -1.66%
For loop (Execution) 22.5±1.23µs 22.5±1.77µs 0.00%
For loop (Full) 204.7±8.94µs 203.4±8.80µs +0.64%
For loop (Lexer) 5.1±0.19µs 5.1±0.17µs 0.00%
For loop (Parser) 14.1±0.37µs 14.5±0.92µs -2.76%
Goal Symbols (Parser) 8.6±0.41µs 8.2±0.35µs +4.88%
Hello World (Lexer) 856.2±89.25ns 835.0±47.54ns +2.54%
Hello World (Parser) 2.2±0.09µs 2.1±0.08µs +4.76%
Long file (Parser) 6.4±0.26ms 6.3±0.12ms +1.59%
Number Object Access (Execution) 3.6±0.13µs 3.6±0.11µs 0.00%
Number Object Access (Full) 186.0±9.81µs 187.9±14.21µs -1.01%
Object Creation (Execution) 4.7±0.26µs 4.7±0.12µs 0.00%
Object Creation (Full) 206.8±7.37µs 218.7±14.93µs -5.44%
RegExp (Execution) 71.9±1.98µs 72.4±2.69µs -0.69%
RegExp (Full) 300.7±16.61µs 295.6±9.19µs +1.73%
RegExp Literal (Execution) 71.6±2.20µs 72.3±2.66µs -0.97%
RegExp Literal (Full) 269.8±8.15µs 273.8±11.59µs -1.46%
RegExp Literal Creation (Execution) 68.7±2.81µs 68.4±3.12µs +0.44%
RegExp Literal Creation (Full) 297.8±7.80µs 296.3±11.96µs +0.51%
Static Object Property Access (Execution) 5.0±0.24µs 5.0±0.18µs 0.00%
Static Object Property Access (Full) 187.1±11.11µs 195.5±9.02µs -4.30%
String Object Access (Execution) 6.7±0.19µs 6.8±0.50µs -1.47%
String Object Access (Full) 190.2±5.67µs 192.0±7.74µs -0.94%
String comparison (Execution) 6.7±0.36µs 6.3±0.35µs +6.35%
String comparison (Full) 187.8±6.01µs 188.2±7.58µs -0.21%
String concatenation (Execution) 5.5±0.28µs 5.1±0.25µs +7.84%
String concatenation (Full) 182.7±4.54µs 185.5±9.93µs -1.51%
String copy (Execution) 4.0±0.12µs 3.8±0.13µs +5.26%
String copy (Full) 179.9±8.44µs 177.6±5.90µs +1.30%
Symbols (Execution) 3.2±0.10µs 3.3±0.18µs -3.03%
Symbols (Full) 172.1±5.50µs 172.3±7.72µs -0.12%

Copy link

@Lan2u Lan2u left a comment

Choose a reason for hiding this comment

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

Can you add a test that demonstrates the correct behavior when the position argument of lastIndexOf is not a number (default to positive infinity)

@HalidOdat HalidOdat force-pushed the fix/string-prototype-lastindexof branch from ac55dd8 to 6ca1354 Compare August 1, 2020 01:05
@github-actions
Copy link

github-actions bot commented Aug 1, 2020

Benchmark for 8a956e0

Click to view benchmark
Test PR Benchmark Master Benchmark %
Arithmetic operations (Execution) 379.5±8.17ns 385.3±10.43ns -1.51%
Arithmetic operations (Full) 172.2±4.20µs 181.6±27.78µs -5.18%
Array access (Execution) 8.8±0.18µs 9.0±0.30µs -2.22%
Array access (Full) 190.7±5.96µs 199.8±14.29µs -4.55%
Array creation (Execution) 3.4±0.11ms 3.4±0.09ms 0.00%
Array creation (Full) 3.7±0.09ms 3.9±0.22ms -5.13%
Array pop (Execution) 1194.3±64.93µs 1224.6±16.81µs -2.47%
Array pop (Full) 1553.7±56.04µs 1569.3±43.90µs -0.99%
Boolean Object Access (Execution) 4.8±0.28µs 4.8±0.13µs 0.00%
Boolean Object Access (Full) 192.3±18.53µs 186.3±3.68µs +3.22%
Create Realm 446.6±6.45ns 453.2±16.36ns -1.46%
Dynamic Object Property Access (Execution) 5.4±0.37µs 5.3±0.06µs +1.89%
Dynamic Object Property Access (Full) 184.4±12.55µs 188.7±4.89µs -2.28%
Expression (Lexer) 2.4±0.06µs 2.5±0.36µs -4.00%
Expression (Parser) 5.6±0.29µs 5.7±0.13µs -1.75%
Fibonacci (Execution) 987.2±29.59µs 980.4±21.65µs +0.69%
Fibonacci (Full) 1179.4±24.83µs 1223.1±52.76µs -3.57%
For loop (Execution) 22.7±0.71µs 23.2±2.27µs -2.16%
For loop (Full) 202.4±8.96µs 206.0±8.15µs -1.75%
For loop (Lexer) 5.3±0.12µs 5.4±0.14µs -1.85%
For loop (Parser) 14.4±0.71µs 14.2±0.44µs +1.41%
Goal Symbols (Parser) 8.8±0.68µs 8.7±0.43µs +1.15%
Hello World (Lexer) 828.3±23.56ns 854.4±29.62ns -3.05%
Hello World (Parser) 2.2±0.06µs 2.3±0.16µs -4.35%
Long file (Parser) 6.5±0.16ms 6.6±0.13ms -1.52%
Number Object Access (Execution) 3.7±0.17µs 3.8±0.09µs -2.63%
Number Object Access (Full) 186.1±11.60µs 185.1±5.05µs +0.54%
Object Creation (Execution) 4.6±0.15µs 4.6±0.07µs 0.00%
Object Creation (Full) 204.2±5.84µs 208.3±10.03µs -1.97%
RegExp (Execution) 70.2±2.34µs 70.8±1.41µs -0.85%
RegExp (Full) 295.2±11.83µs 299.1±11.25µs -1.30%
RegExp Literal (Execution) 70.0±1.99µs 71.2±2.49µs -1.69%
RegExp Literal (Full) 260.5±6.16µs 263.8±9.19µs -1.25%
RegExp Literal Creation (Execution) 67.2±1.99µs 69.9±7.56µs -3.86%
RegExp Literal Creation (Full) 289.2±6.88µs 292.0±8.70µs -0.96%
Static Object Property Access (Execution) 4.8±0.16µs 4.8±0.18µs 0.00%
Static Object Property Access (Full) 183.0±4.51µs 188.9±7.95µs -3.12%
String Object Access (Execution) 6.8±0.15µs 6.9±0.36µs -1.45%
String Object Access (Full) 185.1±6.92µs 189.1±3.48µs -2.12%
String comparison (Execution) 6.3±0.15µs 6.2±0.10µs +1.61%
String comparison (Full) 184.9±8.33µs 188.7±3.93µs -2.01%
String concatenation (Execution) 5.2±0.31µs 5.1±0.13µs +1.96%
String concatenation (Full) 178.5±2.13µs 187.8±10.84µs -4.95%
String copy (Execution) 3.8±0.10µs 3.8±0.06µs 0.00%
String copy (Full) 179.4±7.28µs 179.3±7.86µs +0.06%
Symbols (Execution) 3.2±0.15µs 3.2±0.18µs 0.00%
Symbols (Full) 169.7±3.37µs 176.5±6.53µs -3.85%

@HalidOdat HalidOdat requested a review from Lan2u August 1, 2020 01:45
@HalidOdat HalidOdat merged commit 7f2ea6f into master Aug 1, 2020
@HalidOdat HalidOdat deleted the fix/string-prototype-lastindexof branch August 1, 2020 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working builtins PRs and Issues related to builtins/intrinsics execution Issues or PRs related to code execution performance Performance related changes and issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants