Get rid of redundant viper.Get* calls #2728

n10v · 2016-11-23T20:37:07Z

I was just interested, how often hugo calls viper.Get* with specific keys. The results are:
examples/blog

  17 uglyurls
  17 removepathaccents
  17 paginatepath
  17 multilingual
  17 disablepathtolower
  17 defaultcontentlanguageinsubdir
  17 defaultcontentlanguage
  17 currentcontentlanguage
  17 canonifyurls
  13 currentContentLanguage
  10 rssuri
   3 taxonomies
   2 blackfriday
   2 CurrentContentLanguage
   1 title
   1 social
   1 sectionpagesmenu
   1 preservetaxonomynames
   1 menu
   1 languagecode
   1 googleanalytics
   1 disqusshortname
   1 copyright
   1 author

Built site for language en:
0 draft content
0 future content
0 expired content
13 pages created
0 non-page files copied
0 paginator pages created
4 tags created
3 categories created

examples/multilingual

  23 uglyurls
  23 removepathaccents
  23 paginatepath
  23 multilingual
  23 disablepathtolower
  23 defaultcontentlanguageinsubdir
  23 defaultcontentlanguage
  23 currentcontentlanguage
  23 canonifyurls
  15 currentContentLanguage
  10 blackfriday
   8 rssuri
   5 taxonomies
   4 CurrentContentLanguage
   2 social
   2 sectionpagesmenu
   2 preservetaxonomynames
   2 languagecode
   2 googleanalytics
   2 disqusshortname
   2 copyright
   2 author

Built site for language en:
0 draft content
0 future content
0 expired content
9 pages created
0 non-page files copied
0 paginator pages created
1 groups created
Built site for language et:
0 draft content
0 future content
0 expired content
9 pages created
0 non-page files copied
0 paginator pages created
1 groups created

docs

45925 sectionpagesmenu
 434 uglyurls
 434 removepathaccents
 434 paginatepath
 434 multilingual
 434 disablepathtolower
 434 defaultcontentlanguageinsubdir
 434 defaultcontentlanguage
 434 currentcontentlanguage
 434 canonifyurls
 430 currentContentLanguage
 206 blackfriday
  72 rssuri
   2 taxonomies
   2 CurrentContentLanguage
   1 title
   1 social
   1 preservetaxonomynames
   1 menu
   1 languagecode
   1 googleanalytics
   1 disqusshortname
   1 copyright

Built site for language en:
0 draft content
0 future content
0 expired content
277 pages created
0 non-page files copied
0 paginator pages created
0 groups created
58 tags created

version of hugo: dev dbb0c1c

I think, what the most of these accesses are redundant and could be removed, so the performance will be better. And the repeating helpers.AbsPathify calls should also be considered.

The text was updated successfully, but these errors were encountered:

bep · 2016-11-23T20:57:17Z

45925 sectionpagesmenu

Curious. This is the HasMenuCurrent, I guess. I will get rid of that one right away. This is useful.

See gohugoio#2728

bep · 2016-11-23T21:17:48Z

That said, if you look past the sectionPagesMenu setting, which I would say is a bug, the others are also low in the big picture. It isn't a map lookup, but still fairly cheap -- but it adds up I guess. We should find a clean way to fix this. This is possible related to the "global refactor" issue.

n10v · 2016-11-23T21:19:03Z

Yes, I agree with you, it's cheap, but could be better :)
I'll try to get rid of some obvious repeats.

n10v · 2016-11-24T08:06:48Z

How about a middle layer between hugo and viper like a part of "global refactor"? It will be a separate package and could be named like cache, config or global. It will hold all global typed values. For example:

package global

import (
    ... dependencies
)

var (
    absContentDir   string
    baseURL         string
    canonifyURLs    bool
    currentLang     hugolib.Language // with this property `helpers.Config` could be removed
    templateBaseURL template.URL
    uglyURLs        bool
    ... other values
)

func AbsContentDir() string {
    if absContentDir == "" {
        absContentDir = helpers.AbsPathify(viper.GetString("contentDir"))
    }
    return absContentDir
}
... and so on

And if developer needs some config data, he/she should use either this package for global values or global.CurrentLang.Get*/lang.Get* for language-specific data. And he/she mustn't use directly viper.

This can improve performance, because of decreasing of viper accesses and AbsPathify calls and type casts. It will simplify the hugo structure too.

bep · 2016-11-24T08:43:24Z

It shouldn't be a global, but what you talk about kind of exist already, see PathSpec which is named after what it contains. I added that to get rid of the most glowing use of Viper ...

So your idea is very good, but the implementation has room for improvement: We don't want more globals in Hugo, and this should not be lazily initialized (if absContentDir == "") -- that would have needed a sync.Once to not be racy and to be correct (what if absContentDir is defined as ""?).

We may put the PathSpec and friends into a separate package at some point, but I think we should follow up on that pattern. So I think that a PR to solve this issue would have to:

Use PathSpec where relevant
Add more *Spec types where needed

n10v · 2016-11-24T08:54:50Z

Oh, I didn't see PathSpec structure. I'm just in explore of hugo source code, so I can't know some hugo dev features. But really we should use PathSpec where possible. I'll make a PR these days to enforce PathSpec usage and add more *Specs

bep · 2016-11-24T08:59:31Z

PathSpec was pretty recently added.

n10v · 2016-11-24T09:00:44Z

But to implement this, one should improve ConfigProvider structure and his methods, because there are a lot of TODOs and workarounds, so I can't fully understand it.

bep · 2016-11-24T09:25:08Z

The TODOS are mostly about getting rid of all the globals, which is a bigger job (and a separate issue). The current ConfigProvider works fine for this task. If you don't understand it, then just leave it be as is. We can fix it eventually.

See #2728

n10v · 2016-11-24T14:33:13Z

ConfigProvider really complicates the understanding of architecture of hugo. We should get rid of it. And we can create PathSpecs not from ConfigProviders, but from Languages. But it's another issue at all.
But the PathSpec is a really good thing. We should improve and enforce it. But the problem is, what many methods of PathSpec are defined in helpers/url.go and helpers/path.go and it's not so convenient. Maybe one should move to helpers/pathspec.go. I think, it would be clearer and developer can have a whole picture of PathSpec's functions.

n10v · 2016-11-24T15:05:32Z

The problem is, what some global variables (like canonifyURLs, baseURL, defaultContentLanguage and so on) are used in many places. If you don't want to use another global, I have no idea, how could it be implemented. Maybe we should unify all the globals (and which globals exactly do you mean?) and store all methods with globals values inside one package.

n10v · 2017-02-19T10:45:19Z

Update:
examples/blog

  69 baseurl
  39 theme
  38 relativeurls
  30 canonifyurls
  13 disablerss
  10 rssuri
   9 rsslimit
   8 defaultcontentlanguage
   5 publishdir
   5 languages
   5 builddrafts
   4 taxonomies
   4 pygmentscodefences
   4 ignorefiles
   4 defaultextension
   4 defaultContentLanguage
   4 buildfuture
   4 buildexpired
   3 workingDir
   3 multilingual
   3 blackfriday
   3 baseURL
   2 uglyurls
   2 uglyURLs
   2 themesDir
   2 staticdir
   2 staticDir
   2 removePathAccents
   2 paginatePath
   2 layoutdir
   2 layoutDir
   2 hascjklanguage
   2 footnotereturnlinkcontents
   2 footnoteanchorprefix
   2 enableemoji
   2 disablePathToLower
   2 defaultcontentlanguageinsubdir
   2 defaultContentLanguageInSubdir
   2 contentdir
   2 canonifyURLs
   1 workingdir
   1 verbose
   1 title
   1 themesdir
   1 social
   1 sitemap
   1 sectionpagesmenu
   1 removepathaccents
   1 relativeURLs
   1 publishDir
   1 preservetaxonomynames
   1 pluralizelisttitles
   1 permalinks
   1 params
   1 paginatepath
   1 noTimes
   1 noChmod
   1 menu
   1 logFile
   1 languagecode
   1 i18ndir
   1 googleanalytics
   1 enablerobotstxt
   1 enablemissingtranslationplaceholders
   1 enablegitinfo
   1 disqusshortname
   1 disablesitemap
   1 disablepathtolower
   1 disablekinds
   1 disablehugogeneratorinject
   1 disable404
   1 datadir
   1 copyright
   1 cleanDestinationDir
   1 cacheDir
   1 author

examples/multilingual

  51 relativeurls
  47 baseurl
  44 canonifyurls
  42 theme
  23 languages
  22 builddrafts
  20 buildfuture
  20 buildexpired
  18 disablerss
  16 ignorefiles
  11 blackfriday
  10 usemodtimeasfallback
  10 publishdir
  10 hascjklanguage
  10 footnotereturnlinkcontents
  10 footnoteanchorprefix
  10 enableemoji
   8 taxonomies
   8 rssuri
   7 defaultcontentlanguage
   6 rsslimit
   5 sitemap
   4 uglyurls
   4 multilingual
   4 defaultextension
   4 defaultcontentlanguageinsubdir
   3 workingDir
   3 staticdir
   3 layoutdir
   3 disablesitemap
   3 defaultContentLanguage
   3 baseURL
   2 workingdir
   2 uglyURLs
   2 themesdir
   2 themesDir
   2 staticDir
   2 social
   2 sectionpagesmenu
   2 removepathaccents
   2 removePathAccents
   2 preservetaxonomynames
   2 pluralizelisttitles
   2 permalinks
   2 params
   2 paginatepath
   2 paginatePath
   2 layoutDir
   2 languagecode
   2 googleanalytics
   2 enablerobotstxt
   2 disqusshortname
   2 disablepathtolower
   2 disablekinds
   2 disablehugogeneratorinject
   2 disablePathToLower
   2 disable404
   2 defaultContentLanguageInSubdir
   2 copyright
   2 contentdir
   2 canonifyURLs
   2 author
   1 verbose
   1 relativeURLs
   1 publishDir
   1 noTimes
   1 noChmod
   1 logFile
   1 i18ndir
   1 enablemissingtranslationplaceholders
   1 enablegitinfo
   1 datadir
   1 cleanDestinationDir
   1 cacheDir

docs

10831 canonifyurls
 757 relativeurls
 596 pygmentscodefences
 482 theme
 441 baseurl
 434 defaultcontentlanguage
 431 languages
 429 builddrafts
 428 buildfuture
 428 buildexpired
 374 defaultextension
 301 disablerss
 231 ignorefiles
 215 blackfriday
 214 hascjklanguage
 214 footnotereturnlinkcontents
 214 footnoteanchorprefix
 214 enableemoji
  88 rssuri
  87 rsslimit
  11 pluralizelisttitles
   5 publishdir
   4 taxonomies
   4 defaultContentLanguage
   3 workingDir
   3 multilingual
   3 contentdir
   3 baseURL
   2 workingdir
   2 uglyurls
   2 uglyURLs
   2 themesDir
   2 staticdir
   2 staticDir
   2 removePathAccents
   2 paginatePath
   2 layoutdir
   2 layoutDir
   2 disablePathToLower
   2 defaultcontentlanguageinsubdir
   2 defaultContentLanguageInSubdir
   2 canonifyURLs
   1 verbose
   1 title
   1 themesdir
   1 social
   1 sitemap
   1 sectionpagesmenu
   1 removepathaccents
   1 relativeURLs
   1 publishDir
   1 preservetaxonomynames
   1 permalinks
   1 params
   1 paginatepath
   1 noTimes
   1 noChmod
   1 menu
   1 logFile
   1 languagecode
   1 i18ndir
   1 googleanalytics
   1 enablerobotstxt
   1 enablemissingtranslationplaceholders
   1 enablegitinfo
   1 disqusshortname
   1 disablesitemap
   1 disablepathtolower
   1 disablekinds
   1 disablehugogeneratorinject
   1 disable404
   1 datadir
   1 copyright
   1 cleanDestinationDir
   1 cacheDir

version of hugo: dev 2ea242d

bep · 2017-02-19T10:55:29Z

This is very useful. We should do more, but I'd welcomed a PR for Hugo 0.19 that fixes

#3060

n10v · 2017-02-19T10:56:48Z

I will try to work in this direction

n10v · 2017-02-19T20:37:00Z

@bep, I just want to know your opinion about adding method BaseURL to PathSpec:

// BaseURL just returns the baseURL value.
func (p *PathSpec) BaseURL() string {                                                             
  return p.baseURL                                                                                
}

baseURL is in top 5 most used key in every project above.

Enforce usage of PathSpec Fixes gohugoio#3060 Updates gohugoio#2728

Enforce usage of PathSpec Fixes #3060 Updates #2728

n10v · 2017-04-06T15:22:37Z

Building https://github.com/rdwatters/hugo-docs-concept:

55252 footnotereturnlinkcontents
55252 footnoteanchorprefix
54888 blackfriday
 962 publishdir
 749 pygmentscodefences
 650 defaultcontentlanguage
 645 languages
 640 builddrafts
 638 buildfuture
 638 buildexpired
 342 ignorefiles
 315 hascjklanguage
 313 enableemoji
 261 rsslimit
 261 disablerss
   9 rssuri
   5 defaultContentLanguage
   4 uglyurls
   4 theme
   4 taxonomies
   4 multilingual
   4 ignorecache
   4 defaultcontentlanguageinsubdir
   4 canonifyurls
   4 cachedir
   4 baseurl
   3 workingdir
   3 themesdir
   3 staticdir
   3 removepathaccents
   3 paginatepath
   3 layoutdir
   3 disablepathtolower
   2 workingDir
   2 themesDir
   2 contentdir
   1 verbose
   1 uglyURLs
   1 title
   1 staticDir
   1 social
   1 sitemap
   1 sectionpagesmenu
   1 removePathAccents
   1 relativeurls
   1 relativeURLs
   1 publishDir
   1 preservetaxonomynames
   1 pluralizelisttitles
   1 permalinks
   1 params
   1 paginatePath
   1 noTimes
   1 noChmod
   1 menu
   1 logFile
   1 layoutDir
   1 languagecode
   1 i18ndir
   1 googleanalytics
   1 enablerobotstxt
   1 enablemissingtranslationplaceholders
   1 enablegitinfo
   1 disqusshortname
   1 disablesitemap
   1 disablekinds
   1 disablehugogeneratorinject
   1 disablePathToLower
   1 disable404
   1 defaultContentLanguageInSubdir
   1 datadir
   1 copyright
   1 cleanDestinationDir
   1 canonifyURLs
   1 cacheDir
   1 baseURL
   1 author

$ hugo version
Hugo Static Site Generator v0.20-DEV darwin/amd64 BuildDate: 2017-04-06T17:18:29+02:00

/cc @bep @rdwatters @moorereason

bep · 2017-07-15T10:55:49Z

This is fixed now.

github-actions · 2022-03-17T02:01:13Z

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

n10v changed the title ~~Get rid of redundant viper accesses~~ Get rid of redundant viper.Get calls Nov 23, 2016

n10v changed the title ~~Get rid of redundant viper.Get calls~~ Get rid of redundant viper.Get* calls Nov 23, 2016

bep added the Enhancement label Nov 23, 2016

bep added a commit to bep/hugo that referenced this issue Nov 23, 2016

hugolib: Avoid repeated Viper loads of sectionPagesMenu

e611881

See gohugoio#2728

bep mentioned this issue Nov 23, 2016

hugolib: Avoid repeated Viper loads of sectionPagesMenu #2729

Merged

bep added a commit that referenced this issue Nov 24, 2016

hugolib: Avoid repeated Viper loads of sectionPagesMenu

1f6e0de

See #2728

n10v mentioned this issue Nov 25, 2016

helpers, hugolib, tpl: Get rid of ConfigProvider #2731

Closed

bep mentioned this issue Feb 19, 2017

Get rid of viper.Get(canonifyurls) #3060

Closed

n10v added a commit to n10v/hugo that referenced this issue Feb 19, 2017

Get rid of some viper.Get* calls

61e5d50

Enforce usage of PathSpec Fixes gohugoio#3060 Updates gohugoio#2728

n10v mentioned this issue Feb 19, 2017

Get rid of some viper.Get* calls #3064

Merged

bep pushed a commit that referenced this issue Feb 20, 2017

Get rid of some viper.Get* calls

7e0fa13

Enforce usage of PathSpec Fixes #3060 Updates #2728

n10v mentioned this issue Mar 24, 2017

create: Delete redundant viper calls #3227

Merged

n10v mentioned this issue Apr 6, 2017

hugolib, tpl: Get rid of some redundant viper calls #3292

Closed

bep closed this as completed Jul 15, 2017

github-actions bot added the Outdated label Mar 17, 2022

github-actions bot locked as resolved and limited conversation to collaborators Mar 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get rid of redundant viper.Get* calls #2728

Get rid of redundant viper.Get* calls #2728

n10v commented Nov 23, 2016 •

edited

bep commented Nov 23, 2016 •

edited

bep commented Nov 23, 2016 •

edited

n10v commented Nov 23, 2016 •

edited

n10v commented Nov 24, 2016 •

edited

bep commented Nov 24, 2016

n10v commented Nov 24, 2016

bep commented Nov 24, 2016

n10v commented Nov 24, 2016

bep commented Nov 24, 2016

n10v commented Nov 24, 2016 •

edited

n10v commented Nov 24, 2016 •

edited

n10v commented Feb 19, 2017 •

edited

bep commented Feb 19, 2017

n10v commented Feb 19, 2017

n10v commented Feb 19, 2017 •

edited

n10v commented Apr 6, 2017

bep commented Jul 15, 2017

github-actions bot commented Mar 17, 2022

Get rid of redundant viper.Get* calls #2728

Get rid of redundant viper.Get* calls #2728

Comments

n10v commented Nov 23, 2016 • edited

bep commented Nov 23, 2016 • edited

bep commented Nov 23, 2016 • edited

n10v commented Nov 23, 2016 • edited

n10v commented Nov 24, 2016 • edited

bep commented Nov 24, 2016

n10v commented Nov 24, 2016

bep commented Nov 24, 2016

n10v commented Nov 24, 2016

bep commented Nov 24, 2016

n10v commented Nov 24, 2016 • edited

n10v commented Nov 24, 2016 • edited

n10v commented Feb 19, 2017 • edited

bep commented Feb 19, 2017

n10v commented Feb 19, 2017

n10v commented Feb 19, 2017 • edited

n10v commented Apr 6, 2017

bep commented Jul 15, 2017

github-actions bot commented Mar 17, 2022

n10v commented Nov 23, 2016 •

edited

bep commented Nov 23, 2016 •

edited

bep commented Nov 23, 2016 •

edited

n10v commented Nov 23, 2016 •

edited

n10v commented Nov 24, 2016 •

edited

n10v commented Nov 24, 2016 •

edited

n10v commented Nov 24, 2016 •

edited

n10v commented Feb 19, 2017 •

edited

n10v commented Feb 19, 2017 •

edited