Skip to content
This repository has been archived by the owner on Jul 9, 2018. It is now read-only.

i18n: Memoize dcnpgettext #101

Merged
merged 2 commits into from
Apr 12, 2018
Merged

i18n: Memoize dcnpgettext #101

merged 2 commits into from
Apr 12, 2018

Conversation

aduth
Copy link
Member

@aduth aduth commented Apr 9, 2018

This pull request seeks to memoize the result of the i18n module's central dcnpgettext function. This improves performance of a simple call to __ by roughly 26000% .

Before:

__ x 145,625 ops/sec ±1.01% (90 runs sampled)

After:

__ x 37,909,434 ops/sec ±0.58% (85 runs sampled)

The root of the performance issue is Jed's Jed.PF.compile function, which compiles a plural forms string into an AST, returning a function which interprets a count value to its plural form. There is no caching for this function, and it is invoked on every call to dcnpgettext for the default locale. The first implementation here simply memoized Jed.PF.compile, which improved performance significantly and had the advantage of likely caching just a single value function result. The main downside is that we'd be modifying the module directly, so any other dependents would inherit the same behavior. The approach here is isolated to the i18n module and is more performant, at the expense of higher memory cost of caching the result of each distinct translation call.

@codecov
Copy link

codecov bot commented Apr 9, 2018

Codecov Report

Merging #101 into master will decrease coverage by 0.47%.
The diff coverage is 16.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #101      +/-   ##
==========================================
- Coverage   64.54%   64.07%   -0.48%     
==========================================
  Files          41       42       +1     
  Lines         598      604       +6     
  Branches      118      119       +1     
==========================================
+ Hits          386      387       +1     
- Misses        170      175       +5     
  Partials       42       42
Impacted Files Coverage Δ
packages/i18n/benchmark/index.js 0% <0%> (ø)
packages/i18n/src/index.js 95% <100%> (+0.26%) ⬆️

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 ddc18d0...0c57f15. Read the comment docs.

@aduth aduth merged commit 2bd43bb into master Apr 12, 2018
@aduth aduth deleted the update/i18n-cache-compile branch April 12, 2018 20:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant