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

various performance optimizations #926

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 39 additions & 7 deletions api/generate.go
Original file line number Diff line number Diff line change
@@ -1,25 +1,27 @@
package api

import (
"fmt"
"syscall"
"time"

"github.com/99designs/gqlgen/codegen"
"github.com/99designs/gqlgen/codegen/config"
"github.com/99designs/gqlgen/plugin"
"github.com/99designs/gqlgen/plugin/modelgen"
"github.com/99designs/gqlgen/plugin/resolvergen"
"github.com/99designs/gqlgen/plugin/schemaconfig"
"github.com/pkg/errors"
"golang.org/x/tools/go/packages"
)

func Generate(cfg *config.Config, option ...Option) error {
timeStartTotal := time.Now()
_ = syscall.Unlink(cfg.Exec.Filename)
_ = syscall.Unlink(cfg.Model.Filename)

plugins := []plugin.Plugin{
schemaconfig.New(),
modelgen.New(),
//modelgen.New(),
resolvergen.New(),
}

Expand All @@ -35,15 +37,40 @@ func Generate(cfg *config.Config, option ...Option) error {
}
}
}

// load configs now to get packages needed for loading
schema, schemaStr, err := cfg.LoadSchema()
if err != nil {
return err
}

err = cfg.Check()
if err != nil {
return err
}

/// WARNING: now we inject builtins before autobinding because autobinding required package paths to be resolved and injecting builtins can add new paths
cfg.InjectBuiltins(schema)

packageNames := append(cfg.AutoBind, cfg.Models.ReferencedPackages()...)
timeStart := time.Now()
pkgs, err := packages.Load(&packages.Config{Mode: packages.LoadTypes | packages.LoadSyntax | packages.NeedName}, packageNames...)
if err != nil {
return errors.Wrap(err, "loading failed")
}
fmt.Println("loading time ", time.Now().Sub(timeStart))

// Merge again now that the generated models have been injected into the typemap
data, err := codegen.BuildData(cfg)
data, err := codegen.BuildData(cfg, pkgs, schema, schemaStr)
if err != nil {
return errors.Wrap(err, "merging failed")
}

if err = codegen.GenerateCode(data); err != nil {
timeStart = time.Now()
if err = codegen.GenerateCode(data, pkgs); err != nil {
return errors.Wrap(err, "generating core failed")
}
fmt.Println("generation time ", time.Now().Sub(timeStart))

for _, p := range plugins {
if mut, ok := p.(plugin.CodeGenerator); ok {
Expand All @@ -54,9 +81,14 @@ func Generate(cfg *config.Config, option ...Option) error {
}
}

if err := validate(cfg); err != nil {
return errors.Wrap(err, "validation failed")
}
/*
timeStart = time.Now()
if err := validate(cfg); err != nil {
return errors.Wrap(err, "validation failed")
}
fmt.Println("validation time ", time.Now().Sub(timeStart))
*/
fmt.Println("total time ", time.Now().Sub(timeStartTotal))

return nil
}
Expand Down
7 changes: 1 addition & 6 deletions codegen/config/binder.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,7 @@ type Binder struct {
References []*TypeReference
}

func (c *Config) NewBinder(s *ast.Schema) (*Binder, error) {
pkgs, err := packages.Load(&packages.Config{Mode: packages.LoadTypes | packages.LoadSyntax}, c.Models.ReferencedPackages()...)
if err != nil {
return nil, err
}

func (c *Config) NewBinder(s *ast.Schema, pkgs []*packages.Package) (*Binder, error) {
mp := map[string]*packages.Package{}
for _, p := range pkgs {
populatePkg(mp, p)
Expand Down
23 changes: 17 additions & 6 deletions codegen/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,21 +385,29 @@ func (c *Config) normalize() error {
return nil
}

func (c *Config) Autobind(s *ast.Schema) error {
// TODO: this has to respect ./..., all, etc.
func (c *Config) isAutobind(pkg *packages.Package) bool {
for _, ab := range c.AutoBind {
if pkg.PkgPath == ab {
return true
}
}
return false
}

func (c *Config) Autobind(s *ast.Schema, ps []*packages.Package) error {
if len(c.AutoBind) == 0 {
return nil
}
ps, err := packages.Load(&packages.Config{Mode: packages.LoadTypes}, c.AutoBind...)
if err != nil {
return err
}

for _, t := range s.Types {
if c.Models.UserDefined(t.Name) {
continue
}

for _, p := range ps {
if !c.isAutobind(p) {
continue
}
if t := p.Types.Scope().Lookup(t.Name); t != nil {
c.Models.Add(t.Name(), t.Pkg().Path()+"."+t.Name())
break
Expand All @@ -417,6 +425,9 @@ func (c *Config) Autobind(s *ast.Schema) error {
}

for _, p := range ps {
if !c.isAutobind(p) {
continue
}
if p.Name != pkg {
continue
}
Expand Down
20 changes: 6 additions & 14 deletions codegen/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/99designs/gqlgen/codegen/config"
"github.com/pkg/errors"
"github.com/vektah/gqlparser/ast"
"golang.org/x/tools/go/packages"
)

// Data is a unified model of the code to be generated. Plugins may modify this structure to do things like implement
Expand Down Expand Up @@ -35,30 +36,21 @@ type builder struct {
Directives map[string]*Directive
}

func BuildData(cfg *config.Config) (*Data, error) {
func BuildData(cfg *config.Config, pkgs []*packages.Package, schema *ast.Schema, schemaStr map[string]string) (*Data, error) {
b := builder{
Config: cfg,
}

b.Schema = schema
b.SchemaStr = schemaStr
var err error
b.Schema, b.SchemaStr, err = cfg.LoadSchema()
if err != nil {
return nil, err
}

err = cfg.Check()
if err != nil {
return nil, err
}

err = cfg.Autobind(b.Schema)
err = cfg.Autobind(b.Schema, pkgs)
if err != nil {
return nil, err
}

cfg.InjectBuiltins(b.Schema)

b.Binder, err = b.Config.NewBinder(b.Schema)
b.Binder, err = b.Config.NewBinder(b.Schema, pkgs)
if err != nil {
return nil, err
}
Expand Down
5 changes: 4 additions & 1 deletion codegen/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,17 @@ package codegen

import (
"github.com/99designs/gqlgen/codegen/templates"
"github.com/99designs/gqlgen/internal/code"
"golang.org/x/tools/go/packages"
)

func GenerateCode(data *Data) error {
func GenerateCode(data *Data, packages []*packages.Package) error {
return templates.Render(templates.Options{
PackageName: data.Config.Exec.Package,
Filename: data.Config.Exec.Filename,
Data: data,
RegionTags: true,
GeneratedHeader: true,
NameForPackage: code.NewNameForPackage(packages),
})
}
23 changes: 13 additions & 10 deletions codegen/templates/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,16 @@ import (
)

type Import struct {
Name string
Path string
Alias string
NameForPackage code.NameForPackage
Name string
Path string
Alias string
}

type Imports struct {
imports []*Import
destDir string
NameForPackage code.NameForPackage
imports []*Import
destDir string
}

func (i *Import) String() string {
Expand Down Expand Up @@ -49,7 +51,7 @@ func (s *Imports) Reserve(path string, aliases ...string) (string, error) {
return "", nil
}

name := code.NameForPackage(path)
name := s.NameForPackage.Get(path)
var alias string
if len(aliases) != 1 {
alias = name
Expand All @@ -69,9 +71,10 @@ func (s *Imports) Reserve(path string, aliases ...string) (string, error) {
}

s.imports = append(s.imports, &Import{
Name: name,
Path: path,
Alias: alias,
NameForPackage: s.NameForPackage,
Name: name,
Path: path,
Alias: alias,
})

return "", nil
Expand All @@ -94,7 +97,7 @@ func (s *Imports) Lookup(path string) string {
}

imp := &Import{
Name: code.NameForPackage(path),
Name: s.NameForPackage.Get(path),
Path: path,
}
s.imports = append(s.imports, imp)
Expand Down
11 changes: 7 additions & 4 deletions codegen/templates/templates.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"text/template"
"unicode"

"github.com/99designs/gqlgen/internal/code"
"github.com/99designs/gqlgen/internal/imports"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -43,6 +44,8 @@ type Options struct {
// Data will be passed to the template execution.
Data interface{}
Funcs template.FuncMap
// Lookups for pre-cached package names
NameForPackage code.NameForPackage
}

// Render renders a gql plugin template from the given Options. Render is an
Expand All @@ -53,7 +56,7 @@ func Render(cfg Options) error {
if CurrentImports != nil {
panic(fmt.Errorf("recursive or concurrent call to RenderToFile detected"))
}
CurrentImports = &Imports{destDir: filepath.Dir(cfg.Filename)}
CurrentImports = &Imports{NameForPackage: cfg.NameForPackage, destDir: filepath.Dir(cfg.Filename)}

// load path relative to calling source file
_, callerFile, _, _ := runtime.Caller(1)
Expand Down Expand Up @@ -143,7 +146,7 @@ func Render(cfg Options) error {
}
CurrentImports = nil

return write(cfg.Filename, result.Bytes())
return write(cfg.Filename, result.Bytes(), cfg.NameForPackage)
}

func center(width int, pad string, s string) string {
Expand Down Expand Up @@ -551,13 +554,13 @@ func render(filename string, tpldata interface{}) (*bytes.Buffer, error) {
return buf, t.Execute(buf, tpldata)
}

func write(filename string, b []byte) error {
func write(filename string, b []byte, nameForPackage code.NameForPackage) error {
err := os.MkdirAll(filepath.Dir(filename), 0755)
if err != nil {
return errors.Wrap(err, "failed to create directory")
}

formatted, err := imports.Prune(filename, b)
formatted, err := imports.Prune(filename, b, nameForPackage)
if err != nil {
fmt.Fprintf(os.Stderr, "gofmt failed on %s: %s\n", filepath.Base(filename), err.Error())
formatted = b
Expand Down
33 changes: 23 additions & 10 deletions internal/code/imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import (
"golang.org/x/tools/go/packages"
)

var nameForPackageCache = sync.Map{}

var gopaths []string

func init() {
Expand Down Expand Up @@ -92,24 +90,39 @@ func ImportPathForDir(dir string) (res string) {

var modregex = regexp.MustCompile("module (.*)\n")

type NameForPackage struct {
cache *sync.Map
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did this need to be moved out of a private global?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically it didn't need to be moved, but it was weird to me to use global state when we were already creating a struct to hold the packages field.

In addition to the fact that global state is a dangerous anti-pattern, I suspect that this would also have caused issues if I wanted to use the same binary to generate multiple graphql servers.

packages []*packages.Package
}

func NewNameForPackage(packages []*packages.Package) NameForPackage {
return NameForPackage{
cache: &sync.Map{},
packages: packages,
}
}

// NameForPackage returns the package name for a given import path. This can be really slow.
func NameForPackage(importPath string) string {
func (n NameForPackage) Get(importPath string) string {
if importPath == "" {
panic(errors.New("import path can not be empty"))
}
if v, ok := nameForPackageCache.Load(importPath); ok {
if v, ok := n.cache.Load(importPath); ok {
return v.(string)
}
importPath = QualifyPackagePath(importPath)
p, _ := packages.Load(&packages.Config{
Mode: packages.NeedName,
}, importPath)
var p *packages.Package
for _, pkg := range n.packages {
if pkg.PkgPath == importPath {
p = pkg
}
}

if len(p) != 1 || p[0].Name == "" {
if p == nil || p.Name == "" {
return SanitizePackageName(filepath.Base(importPath))
}

nameForPackageCache.Store(importPath, p[0].Name)
n.cache.Store(importPath, p.Name)

return p[0].Name
return p.Name
}
5 changes: 5 additions & 0 deletions internal/code/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ func NormalizeVendor(pkg string) string {
// x/tools/packages only supports 'qualified package paths' so this will need to be done prior to calling it
// See https://github.com/golang/go/issues/30289
func QualifyPackagePath(importPath string) string {
// TODO: check if we are in go module mode or not
goModuleMode := true
if goModuleMode {
return importPath
}
wd, _ := os.Getwd()

pkg, err := build.Import(importPath, wd, 0)
Expand Down
Loading