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

Read file input in bytes instead of string #979

Merged
merged 1 commit into from
Dec 18, 2020

Conversation

jevancc
Copy link
Contributor

@jevancc jevancc commented Dec 18, 2020

This Pull Request is a follow-up PR for #915, which closes #876.

This PR may also improve run-time performance slightly since it removes the unnecessary bytes to utf8 chars conversion when reading the files.

It changes the following:

  • Read the file input in bytes (boa_cli)

Except for file input in boa_cli, all the other places still take input in str and convert it to bytes afterward:

boa_cli command prompt

The crate rustyline reads input in strings and takes care of it.

boa_wasm

wasm_bindgen makes JS interface and takes input in strings.

test/benchmark

The benchmark still uses include_str! to read the bench scripts. Since the files are read in compile-time, it does not affect the run-time performance. Keep it as it can also benefit because it checks if there are any invalid chars in the bench scripts in compile time.
Similar reasons for tests. Taking the input in str makes the tests simpler and clearer.

@codecov
Copy link

codecov bot commented Dec 18, 2020

Codecov Report

Merging #979 (345baee) into master (b058b2d) will increase coverage by 0.10%.
The diff coverage is 65.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #979      +/-   ##
==========================================
+ Coverage   59.56%   59.67%   +0.10%     
==========================================
  Files         166      166              
  Lines       10815    10807       -8     
==========================================
+ Hits         6442     6449       +7     
+ Misses       4373     4358      -15     
Impacted Files Coverage Δ
boa_cli/src/main.rs 33.33% <ø> (ø)
boa_tester/src/exec.rs 0.00% <0.00%> (ø)
boa/src/context.rs 61.43% <100.00%> (+0.17%) ⬆️
boa/src/lib.rs 88.46% <100.00%> (+2.09%) ⬆️
...ntax/parser/expression/left_hand_side/arguments.rs 63.15% <0.00%> (+5.26%) ⬆️
...tax/parser/expression/assignment/exponentiation.rs 94.44% <0.00%> (+9.44%) ⬆️
boa/src/syntax/parser/expression/mod.rs 92.53% <0.00%> (+14.05%) ⬆️

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 b058b2d...345baee. Read the comment docs.

@RageKnify RageKnify added the performance Performance related changes and issues label Dec 18, 2020
@Razican Razican added this to the v0.11.0 milestone Dec 18, 2020
@Razican
Copy link
Member

Razican commented Dec 18, 2020

Thanks!

@Razican Razican merged commit 880792e into boa-dev:master Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance related changes and issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle invalid UTF-8 chars in user input instead of panic
3 participants