From 4756ec3cd8ef998f889619fe11be70cc900e2b75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Tue, 23 Apr 2019 19:24:19 +0200 Subject: [PATCH] i18n: Avoid rebuilding the Translations map for every lookup ```bash benchmark old ns/op new ns/op delta BenchmarkI18nTranslate/all-present-4 764 757 -0.92% BenchmarkI18nTranslate/present-in-default-4 2578 1457 -43.48% BenchmarkI18nTranslate/present-in-current-4 764 766 +0.26% BenchmarkI18nTranslate/missing-4 3362 1103 -67.19% BenchmarkI18nTranslate/file-missing-4 4646 3611 -22.28% BenchmarkI18nTranslate/context-provided-4 2013 2014 +0.05% BenchmarkI18nTranslate/same-id-and-translation-4 1961 784 -60.02% BenchmarkI18nTranslate/same-id-and-translation-default-4 3717 1405 -62.20% BenchmarkI18nTranslate/unknown-language-code-4 1775 1787 +0.68% benchmark old allocs new allocs delta BenchmarkI18nTranslate/all-present-4 6 6 +0.00% BenchmarkI18nTranslate/present-in-default-4 16 10 -37.50% BenchmarkI18nTranslate/present-in-current-4 6 6 +0.00% BenchmarkI18nTranslate/missing-4 20 8 -60.00% BenchmarkI18nTranslate/file-missing-4 27 21 -22.22% BenchmarkI18nTranslate/context-provided-4 15 15 +0.00% BenchmarkI18nTranslate/same-id-and-translation-4 12 6 -50.00% BenchmarkI18nTranslate/same-id-and-translation-default-4 22 10 -54.55% BenchmarkI18nTranslate/unknown-language-code-4 13 13 +0.00% benchmark old bytes new bytes delta BenchmarkI18nTranslate/all-present-4 152 152 +0.00% BenchmarkI18nTranslate/present-in-default-4 1144 216 -81.12% BenchmarkI18nTranslate/present-in-current-4 152 152 +0.00% BenchmarkI18nTranslate/missing-4 2008 152 -92.43% BenchmarkI18nTranslate/file-missing-4 1208 600 -50.33% BenchmarkI18nTranslate/context-provided-4 704 704 +0.00% BenchmarkI18nTranslate/same-id-and-translation-4 1080 152 -85.93% BenchmarkI18nTranslate/same-id-and-translation-default-4 2073 216 -89.58% BenchmarkI18nTranslate/unknown-language-code-4 696 696 +0.00% ``` Fixes #5892 --- i18n/i18n.go | 13 ++++++----- i18n/i18n_test.go | 55 ++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/i18n/i18n.go b/i18n/i18n.go index 25f2d300e..5beef8683 100644 --- a/i18n/i18n.go +++ b/i18n/i18n.go @@ -19,6 +19,7 @@ import ( "github.com/gohugoio/hugo/helpers" "github.com/nicksnyder/go-i18n/i18n/bundle" + "github.com/nicksnyder/go-i18n/i18n/translation" ) var ( @@ -64,6 +65,8 @@ func (t Translator) initFuncs(bndl *bundle.Bundle) { t.logger.INFO.Printf("No translation bundle found for default language %q", defaultContentLanguage) } + translations := bndl.Translations() + enableMissingTranslationPlaceholders := t.cfg.GetBool("enableMissingTranslationPlaceholders") for _, lang := range bndl.LanguageTags() { currentLang := lang @@ -82,7 +85,7 @@ func (t Translator) initFuncs(bndl *bundle.Bundle) { // then Tfunc returns translationID itself. // But if user set same translationID and translation, we should check // if it really untranslated: - if isIDTranslated(currentLang, translationID, bndl) { + if isIDTranslated(translations, currentLang, translationID) { return translated } @@ -97,7 +100,7 @@ func (t Translator) initFuncs(bndl *bundle.Bundle) { if translated != translationID { return translated } - if isIDTranslated(defaultContentLanguage, translationID, bndl) { + if isIDTranslated(translations, defaultContentLanguage, translationID) { return translated } } @@ -106,9 +109,9 @@ func (t Translator) initFuncs(bndl *bundle.Bundle) { } } -// If bndl contains the translationID for specified currentLang, +// If the translation map contains translationID for specified currentLang, // then the translationID is actually translated. -func isIDTranslated(lang, id string, b *bundle.Bundle) bool { - _, contains := b.Translations()[lang][id] +func isIDTranslated(translations map[string]map[string]translation.Translation, lang, id string) bool { + _, contains := translations[lang][id] return contains } diff --git a/i18n/i18n_test.go b/i18n/i18n_test.go index 5db539d30..b67cabc55 100644 --- a/i18n/i18n_test.go +++ b/i18n/i18n_test.go @@ -23,18 +23,19 @@ import ( "github.com/gohugoio/hugo/htesting" "github.com/gohugoio/hugo/langs" "github.com/spf13/afero" + "github.com/spf13/viper" "github.com/gohugoio/hugo/deps" "github.com/gohugoio/hugo/config" "github.com/gohugoio/hugo/hugofs" - "github.com/spf13/viper" "github.com/stretchr/testify/require" ) var logger = loggers.NewErrorLogger() type i18nTest struct { + name string data map[string][]byte args interface{} lang, id, expected, expectedFlag string @@ -43,6 +44,7 @@ type i18nTest struct { var i18nTests = []i18nTest{ // All translations present { + name: "all-present", data: map[string][]byte{ "en.toml": []byte("[hello]\nother = \"Hello, World!\""), "es.toml": []byte("[hello]\nother = \"¡Hola, Mundo!\""), @@ -55,6 +57,7 @@ var i18nTests = []i18nTest{ }, // Translation missing in current language but present in default { + name: "present-in-default", data: map[string][]byte{ "en.toml": []byte("[hello]\nother = \"Hello, World!\""), "es.toml": []byte("[goodbye]\nother = \"¡Adiós, Mundo!\""), @@ -67,6 +70,7 @@ var i18nTests = []i18nTest{ }, // Translation missing in default language but present in current { + name: "present-in-current", data: map[string][]byte{ "en.toml": []byte("[goodbye]\nother = \"Goodbye, World!\""), "es.toml": []byte("[hello]\nother = \"¡Hola, Mundo!\""), @@ -79,6 +83,7 @@ var i18nTests = []i18nTest{ }, // Translation missing in both default and current language { + name: "missing", data: map[string][]byte{ "en.toml": []byte("[goodbye]\nother = \"Goodbye, World!\""), "es.toml": []byte("[goodbye]\nother = \"¡Adiós, Mundo!\""), @@ -91,6 +96,7 @@ var i18nTests = []i18nTest{ }, // Default translation file missing or empty { + name: "file-missing", data: map[string][]byte{ "en.toml": []byte(""), }, @@ -102,6 +108,7 @@ var i18nTests = []i18nTest{ }, // Context provided { + name: "context-provided", data: map[string][]byte{ "en.toml": []byte("[wordCount]\nother = \"Hello, {{.WordCount}} people!\""), "es.toml": []byte("[wordCount]\nother = \"¡Hola, {{.WordCount}} gente!\""), @@ -119,6 +126,7 @@ var i18nTests = []i18nTest{ // Same id and translation in current language // https://github.com/gohugoio/hugo/issues/2607 { + name: "same-id-and-translation", data: map[string][]byte{ "es.toml": []byte("[hello]\nother = \"hello\""), "en.toml": []byte("[hello]\nother = \"hi\""), @@ -131,6 +139,7 @@ var i18nTests = []i18nTest{ }, // Translation missing in current language, but same id and translation in default { + name: "same-id-and-translation-default", data: map[string][]byte{ "es.toml": []byte("[bye]\nother = \"bye\""), "en.toml": []byte("[hello]\nother = \"hello\""), @@ -143,6 +152,7 @@ var i18nTests = []i18nTest{ }, // Unknown language code should get its plural spec from en { + name: "unknown-language-code", data: map[string][]byte{ "en.toml": []byte(`[readingTime] one ="one minute read" @@ -159,24 +169,29 @@ other = "{{ .Count }} minuttar lesing"`), }, } -func doTestI18nTranslate(t *testing.T, test i18nTest, cfg config.Provider) string { +func doTestI18nTranslate(t testing.TB, test i18nTest, cfg config.Provider) string { + tp := prepareTranslationProvider(t, test, cfg) + f := tp.t.Func(test.lang) + return f(test.id, test.args) + +} + +func prepareTranslationProvider(t testing.TB, test i18nTest, cfg config.Provider) *TranslationProvider { assert := require.New(t) fs := hugofs.NewMem(cfg) - tp := NewTranslationProvider() for file, content := range test.data { err := afero.WriteFile(fs.Source, filepath.Join("i18n", file), []byte(content), 0755) assert.NoError(err) } + tp := NewTranslationProvider() depsCfg := newDepsConfig(tp, cfg, fs) d, err := deps.New(depsCfg) assert.NoError(err) - assert.NoError(d.LoadResources()) - f := tp.t.Func(test.lang) - return f(test.id, test.args) + return tp } func newDepsConfig(tp *TranslationProvider, cfg config.Provider, fs *hugofs.Fs) deps.DepsCfg { @@ -193,8 +208,7 @@ func newDepsConfig(tp *TranslationProvider, cfg config.Provider, fs *hugofs.Fs) } } -func TestI18nTranslate(t *testing.T) { - var actual, expected string +func getConfig() *viper.Viper { v := viper.New() v.SetDefault("defaultContentLanguage", "en") v.Set("contentDir", "content") @@ -205,6 +219,13 @@ func TestI18nTranslate(t *testing.T) { v.Set("assetDir", "assets") v.Set("resourceDir", "resources") v.Set("publishDir", "public") + return v + +} + +func TestI18nTranslate(t *testing.T) { + var actual, expected string + v := getConfig() // Test without and with placeholders for _, enablePlaceholders := range []bool{false, true} { @@ -221,3 +242,21 @@ func TestI18nTranslate(t *testing.T) { } } } + +func BenchmarkI18nTranslate(b *testing.B) { + v := getConfig() + for _, test := range i18nTests { + b.Run(test.name, func(b *testing.B) { + tp := prepareTranslationProvider(b, test, v) + b.ResetTimer() + for i := 0; i < b.N; i++ { + f := tp.t.Func(test.lang) + actual := f(test.id, test.args) + if actual != test.expected { + b.Fatalf("expected %v got %v", test.expected, actual) + } + } + }) + } + +}