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

Test PR #2 on personal fork for API docs / CI dashboard #12

Merged
merged 91 commits into from
Oct 9, 2020

Conversation

echeran
Copy link
Owner

@echeran echeran commented Oct 9, 2020

Won't be as fully featured as PR #10 because it includes a commit that readies it for upstream (unicode-org/icu4x) that prevents storing benchmark results and prevents pushing benchmark dashboards / API docs to the docs repo on PRs. Storing benchmark results / pushing docs works only on merges to master.

This PR will need to see the difference before and after merging to master exhibited by the GH actions.

sffc and others added 30 commits September 3, 2020 13:39
Also adding new documentation about milestones.
* Switch PluralRules to use DataProvider

* Apply feedback and move data to FsDataProvider in tests directory

* Apply feedback
* Add PluralRules example

* Apply feedback
* Add example for icu_locale

* Remodel the app

* Apply feedback

* Add a note
* Add subset of dates data to DataProvider

* Apply feedback

* Make some fields mandatory

* Make StandAlone Widths optional

* Make short width optional as per UTS 35

* Readd skipping if short width is none

* Re-add default to empty values for Invariant
…#256) (unicode-org#258)

* Add support for Dates to CLDRJsonDataProvider

* Apply feedback
unicode-org#261)

* Add support for Dates to CLDRJsonDataProvider

* Apply feedback

* Add Dates to FsDataProviderExport
Adds specific instructions about hiding new dependencies or debug code
behind new features.

Closes unicode-org#219
* Implemented from_str for FixedDecimal and added two tests for it, added Syntax to enum Error for FixedDecimal

* Handled the case when the input string has only 0 in it and Added tests for it. Added test for test_from_str_usize_limits

* correction for clipping errors and format error

* Corrections for clipping errors and format error

* fixing merging issues

* fixing merging issues

* run cargo fmt

* Fixing two clipping errors

* cleaning code and adding test cases

* Added explanations for Error::Syntax

* resolving warnings related to my code

* corrections for documentation of Error::Syntax

* fixing clipping errors

* fixing new clipping errors

* Applied cosmetic suggestions of Shane

* deleted not necessary check on length of FixedDecimal::digits

* adding from_str to benches
* Makes `PluralOperands::n` into a getter

The field `n` is not used for plural selection, and may cause
precision loss in cases of conversion.  So we stop keeping in
around and compute it on demand instead.

Fixes unicode-org#283

* fixup: simplify eval logic, clean up lint
* Implements From<FixedDecimal> for PluralOperands

This will allow us to convert FixedDecimal representation without loss
of precision into PluralOperands, for correct plural selection.

For example, a number `25` may sometimes be pluralized differently from
`25.0`.

Added the benchmarks for the conversion, though without a baseline it is
not just as useful yet.  (Until we try to reimplement.)

See issue unicode-org#190.

* Rewrite eq() to branchless

* Adds a test for eq()

We needed to add a custom implementation for eq() to account
for loss of precision in PluralOperands.

* fixup:first

* Clean up the code for From

* Adds more illustrative naming in From

* Makes clippy happy

* Pulls num_fractional_digits out of the loop

It is possible to compute it based on the low end of the magnitude
range.  No performance change per benchmark.

* fixup: benchmark was wrong

* fixup: moves new operand tests to json

Defines new tests for the data model for conversion, and places them
into the JSON files instead of inline with the tests.

* fixup: moves the benchmark loop in

This allows criterion to run the benchmark loop
with a specific time limit.

* fixup: formatting

* fixup: adds individual sample measurements

Helps smoke out specific performance regressions.
* Some speed improvements to FixedDecimal parsing

* Change str iteration into bytes iteration.  Since the input alphabet
  is only single bytes, this should be OK.

* Use `FixedDecimal::default()` instead of zero for initialization.

Seems to make a 7% improvement in some microbenchmarks, going roughly
from 35ns to 31ns.

```
from_string/0012.3400   time:   [30.871 ns 30.887 ns 30.907 ns]
                        change: [-9.2063% -9.1540% -9.1054%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  4 (4.00%) high mild
  1 (1.00%) high severe
from_string/00.0012216734340
                        time:   [67.715 ns 67.778 ns 67.844 ns]
                        change: [-1.4520% -0.4672% +0.4506%] (p = 0.35 > 0.05)
                        No change in performance detected.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild
from_string/00002342561123400.0
                        time:   [72.554 ns 72.708 ns 72.854 ns]
                        change: [-1.1182% -0.8930% -0.6325%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) low mild
  1 (1.00%) high mild
from_string/-00123400   time:   [30.959 ns 30.971 ns 30.984 ns]
                        change: [-10.060% -9.6862% -9.4502%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
  1 (1.00%) low mild
  2 (2.00%) high mild
  1 (1.00%) high severe
from_string/922337203685477580898230948203840239384.9823094820384023938423424
                        time:   [215.19 ns 215.40 ns 215.63 ns]
                        change: [+3.1132% +3.6683% +4.0742%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 7 outliers among 100 measurements (7.00%)
  2 (2.00%) low mild
  3 (3.00%) high mild
  2 (2.00%) high severe
from_string/0.000000001 time:   [29.235 ns 29.268 ns 29.308 ns]
                        change: [-9.7930% -9.7035% -9.6051%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  1 (1.00%) low mild
  2 (2.00%) high mild
  4 (4.00%) high severe
from_string/1000000001  time:   [60.990 ns 61.038 ns 61.088 ns]
                        change: [+5.0850% +5.2075% +5.3511%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 4 outliers among 100 measurements (4.00%)
  3 (3.00%) high mild
  1 (1.00%) high severe
from_string/0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000...
                        time:   [71.210 us 71.251 us 71.306 us]
                        change: [-1.7824% -1.7383% -1.6889%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 13 outliers among 100 measurements (13.00%)
  6 (6.00%) high mild
  7 (7.00%) high severe

```

* fixup: remove unneeded mutability removal

* fixup: remove unneeded mutability
* Add DateTimeFormat

* Add Preferences to Options and remove leftovers from Skeleton building

* Add fixtures and data driven tests

* Add docs and clean up the code

* Clippy!

* More clippy

* More clippy

* Remove some monads that are not needed anymore and remove all unwraps from src

* Apply reviewers feedback

* Apply reviewers feedback

* Improve format_number to comply with UTS 35 and fix clippy

* Improve pattern parsing

* Switch parser pattern to Bytes

* Apply feedback

* Add an example
* Move icu_num_utils to utils/num

* Add github action

* Rename to fixed-decimal

* Fix a spelling
@coveralls
Copy link

coveralls commented Oct 9, 2020

Pull Request Test Coverage Report for Build f7cac272504388b5f14179e14382d568c55920a1-PR-12

  • 803 of 1071 (74.98%) changed or added relevant lines in 28 files are covered.
  • 5 unchanged lines in 5 files lost coverage.
  • Overall coverage decreased (-1.9%) to 77.652%

Changes Missing Coverage Covered Lines Changed/Added Lines %
components/data-provider/src/structs/plurals.rs 0 1 0.0%
components/fs-data-provider/src/fs_data_provider.rs 7 8 87.5%
components/cldr-json-data-provider/src/cldr_paths.rs 0 2 0.0%
components/datetime/src/pattern/mod.rs 52 54 96.3%
components/fs-data-provider/src/bin/icu4x-cldr-export.rs 0 4 0.0%
components/datetime/src/fields/length.rs 7 12 58.33%
components/datetime/src/options/style.rs 6 12 50.0%
components/datetime/src/pattern/error.rs 0 6 0.0%
components/pluralrules/src/error.rs 3 9 33.33%
components/pluralrules/src/data.rs 37 46 80.43%
Files with Coverage Reduction New Missed Lines %
components/cldr-json-data-provider/src/reader.rs 1 0%
components/cldr-json-data-provider/src/transform/mod.rs 1 0%
components/data-provider/src/data_key.rs 1 85.0%
components/fs-data-provider/src/bin/icu4x-cldr-export.rs 1 0%
components/pluralrules/src/operands.rs 1 83.75%
Totals Coverage Status
Change from base Build 3ebc97d7ccc20b02396b07312987d6474d627463: -1.9%
Covered Lines: 2891
Relevant Lines: 3723

💛 - Coveralls

@echeran echeran merged commit 7b81283 into master Oct 9, 2020
@echeran echeran deleted the api-docs-ci-dash-test-v2 branch June 17, 2021 03:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants