Skip to content

Load contents of source files only when needed #559

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

Merged
merged 4 commits into from
Jan 17, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Load sketch file contents only when needed
Previously, the full contents of *all* sketch files would be loaded into
memory. This includes all source and header files inside the sketch
directory, even when they will not even be compiled (e.g.
subdirectories other than src). In practice, only the .ino file contents
will actually be used, so these are now read on demand.

Note that when copying the sketch into the build directory, the contents
of all these sketch files *is* used, but that code (`writeIfDifferent()`
in `arduino/builder/sketch.go`) already did not use the preloaded data
but read the file contents when copying.

For small sketches, this does not make much of a difference, but bigger
sketches, especially when they include libraries, core definitions,
tools, examples, documentation, etc. the memory usage can quite explode,
for no good reason.
  • Loading branch information
matthijskooijman committed Jan 16, 2020
commit 903247d0e15e83ec491a54b095683e8eaf6d3fd0
28 changes: 16 additions & 12 deletions arduino/sketch/sketch.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,32 @@ import (

// Item holds the source and the path for a single sketch file
type Item struct {
Path string
Source []byte
Path string
}

// NewItem reads the source code for a sketch item and returns an
// Item instance
func NewItem(itemPath string) (*Item, error) {
func NewItem(itemPath string) *Item {
return &Item{itemPath}
}

// GetSourceBytes reads the item file contents and returns it as bytes
func (i *Item) GetSourceBytes() ([]byte, error) {
// read the file
source, err := ioutil.ReadFile(itemPath)
source, err := ioutil.ReadFile(i.Path)
if err != nil {
return nil, errors.Wrap(err, "error reading source file")
}

return &Item{itemPath, source}, nil
return source, nil
}

// GetSourceStr returns the Source contents in string format
// GetSourceStr reads the item file contents and returns it as a string
func (i *Item) GetSourceStr() (string, error) {
return string(i.Source), nil
source, err := i.GetSourceBytes()
if err != nil {
return "", err
}
return string(source), nil
}

// ItemByPath implements sort.Interface for []Item based on
Expand Down Expand Up @@ -73,10 +80,7 @@ func New(sketchFolderPath, mainFilePath, buildPath string, allFilesPaths []strin
pathToItem := make(map[string]*Item)
for _, p := range allFilesPaths {
// create an Item
item, err := NewItem(p)
if err != nil {
return nil, errors.Wrap(err, "error creating the sketch")
}
item := NewItem(p)

if p == mainFilePath {
// store the main sketch file
Expand Down
22 changes: 13 additions & 9 deletions arduino/sketch/sketch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,26 @@ import (

func TestNewItem(t *testing.T) {
sketchItem := filepath.Join("testdata", t.Name()+".ino")
item, err := sketch.NewItem(sketchItem)
assert.Nil(t, err)
item := sketch.NewItem(sketchItem)
assert.Equal(t, sketchItem, item.Path)
assert.Equal(t, []byte(`#include <testlib.h>`), item.Source)
assert.Equal(t, "#include <testlib.h>", item.GetSourceStr())
sourceBytes, err := item.GetSourceBytes()
assert.Nil(t, err)
assert.Equal(t, []byte(`#include <testlib.h>`), sourceBytes)
sourceStr, err := item.GetSourceStr()
assert.Nil(t, err)
assert.Equal(t, "#include <testlib.h>", sourceStr)

item, err = sketch.NewItem("doesnt/exist")
assert.Nil(t, item)
item = sketch.NewItem("doesnt/exist")
sourceBytes, err = item.GetSourceBytes()
assert.Nil(t, sourceBytes)
assert.NotNil(t, err)
}

func TestSort(t *testing.T) {
items := []*sketch.Item{
&sketch.Item{"foo", nil},
&sketch.Item{"baz", nil},
&sketch.Item{"bar", nil},
&sketch.Item{"foo"},
&sketch.Item{"baz"},
&sketch.Item{"bar"},
}

sort.Sort(sketch.ItemByPath(items))
Expand Down
6 changes: 1 addition & 5 deletions legacy/builder/sketch_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,7 @@ func collectAllSketchFiles(from *paths.Path) (paths.PathList, error) {
func makeSketch(sketchLocation *paths.Path, allSketchFilePaths paths.PathList, buildLocation *paths.Path, logger i18n.Logger) (*types.Sketch, error) {
sketchFilesMap := make(map[string]types.SketchFile)
for _, sketchFilePath := range allSketchFilePaths {
source, err := sketchFilePath.ReadFile()
if err != nil {
return nil, i18n.WrapError(err)
}
sketchFilesMap[sketchFilePath.String()] = types.SketchFile{Name: sketchFilePath, Source: string(source)}
sketchFilesMap[sketchFilePath.String()] = types.SketchFile{Name: sketchFilePath}
}

mainFile := sketchFilesMap[sketchLocation.String()]
Expand Down
19 changes: 6 additions & 13 deletions legacy/builder/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,7 @@ func (f *SourceFile) DepfilePath(ctx *Context) *paths.Path {
}

type SketchFile struct {
Name *paths.Path
Source string
Name *paths.Path
}

type SketchFileSortByName []SketchFile
Expand All @@ -114,20 +113,17 @@ func SketchToLegacy(sketch *sketch.Sketch) *Sketch {
s := &Sketch{}
s.MainFile = SketchFile{
paths.New(sketch.MainFile.Path),
string(sketch.MainFile.Source),
}

for _, item := range sketch.OtherSketchFiles {
s.OtherSketchFiles = append(s.OtherSketchFiles, SketchFile{
paths.New(item.Path),
string(item.Source),
})
}

for _, item := range sketch.AdditionalFiles {
s.AdditionalFiles = append(s.AdditionalFiles, SketchFile{
paths.New(item.Path),
string(item.Source),
})
}

Expand All @@ -137,22 +133,19 @@ func SketchToLegacy(sketch *sketch.Sketch) *Sketch {
func SketchFromLegacy(s *Sketch) *sketch.Sketch {
others := []*sketch.Item{}
for _, f := range s.OtherSketchFiles {
if i, err := sketch.NewItem(f.Name.String()); err == nil {
others = append(others, i)
}
i := sketch.NewItem(f.Name.String())
others = append(others, i)
}

additional := []*sketch.Item{}
for _, f := range s.AdditionalFiles {
if i, err := sketch.NewItem(f.Name.String()); err == nil {
additional = append(additional, i)
}
i := sketch.NewItem(f.Name.String())
additional = append(additional, i)
}

return &sketch.Sketch{
MainFile: &sketch.Item{
Path: s.MainFile.Name.String(),
Source: []byte(s.MainFile.Source),
Path: s.MainFile.Name.String(),
},
LocationPath: s.MainFile.Name.Parent().String(),
OtherSketchFiles: others,
Expand Down