Skip to content

Commit

Permalink
Reuse (cache) object archives in large sketch projects (#2464)
Browse files Browse the repository at this point in the history
* Reuse archiveCompiledFiles helper for long commandline shrink

Since archiveCompiledFiles already handles hot cache correctly, this avoids objs.a being rebuilt even if files don't change.

Would be ideal if PathList could expose a generic Filter API (to get rid of the "duplicated" filter)

* Upgrade go-paths / remove duplicate filter function

* Consider existing archives during the build

* Simplified archiveCompiledFiles function signature

It doesn't make sense anymore to keep path and filename separated.

* Added integration test

---------

Co-authored-by: Cristian Maglie <[email protected]>
  • Loading branch information
facchinm and cmaglie committed Dec 20, 2023
1 parent 77222ec commit c2678cf
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 35 deletions.
2 changes: 1 addition & 1 deletion .licenses/go/github.com/arduino/go-paths-helper.dep.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
name: github.com/arduino/go-paths-helper
version: v1.9.2
version: v1.11.0
type: go
summary:
homepage: https://pkg.go.dev/github.com/arduino/go-paths-helper
Expand Down
4 changes: 1 addition & 3 deletions arduino/builder/archive_compiled_files.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ import (
)

// ArchiveCompiledFiles fixdoc
func (b *Builder) archiveCompiledFiles(buildPath *paths.Path, archiveFile *paths.Path, objectFilesToArchive paths.PathList) (*paths.Path, error) {
archiveFilePath := buildPath.JoinPath(archiveFile)

func (b *Builder) archiveCompiledFiles(archiveFilePath *paths.Path, objectFilesToArchive paths.PathList) (*paths.Path, error) {
if b.onlyUpdateCompilationDatabase {
if b.logger.Verbose() {
b.logger.Info(tr("Skipping archive creation of: %[1]s", archiveFilePath))
Expand Down
2 changes: 1 addition & 1 deletion arduino/builder/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func (b *Builder) compileCore() (*paths.Path, paths.PathList, error) {
return nil, nil, errors.WithStack(err)
}

archiveFile, err := b.archiveCompiledFiles(b.coreBuildPath, paths.New("core.a"), coreObjectFiles)
archiveFile, err := b.archiveCompiledFiles(b.coreBuildPath.Join("core.a"), coreObjectFiles)
if err != nil {
return nil, nil, errors.WithStack(err)
}
Expand Down
2 changes: 1 addition & 1 deletion arduino/builder/libraries.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func (b *Builder) compileLibrary(library *libraries.Library, includes []string)
return nil, errors.WithStack(err)
}
if library.DotALinkage {
archiveFile, err := b.archiveCompiledFiles(libraryBuildPath, paths.New(library.DirName+".a"), libObjectFiles)
archiveFile, err := b.archiveCompiledFiles(libraryBuildPath.Join(library.DirName+".a"), libObjectFiles)
if err != nil {
return nil, errors.WithStack(err)
}
Expand Down
45 changes: 21 additions & 24 deletions arduino/builder/linker.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,34 +54,31 @@ func (b *Builder) link() error {
// it may happen that a subdir/spi.o inside the archive may be overwritten by a anotherdir/spi.o
// because thery are both named spi.o.

properties := b.buildProperties.Clone()
archives := paths.NewPathList()
// Put all the existing archives apart from the other object files
existingArchives := objectFiles.Clone()
existingArchives.FilterSuffix(".a")
objectFiles.FilterOutSuffix(".a")

// Generate an archive for each directory from the remaining object files
newArchives := paths.NewPathList()
for _, object := range objectFiles {
if object.HasSuffix(".a") {
archives.Add(object)
continue
}
archive := object.Parent().Join("objs.a")
if !archives.Contains(archive) {
archives.Add(archive)
// Cleanup old archives
_ = archive.Remove()
}
properties.Set("archive_file", archive.Base())
properties.SetPath("archive_file_path", archive)
properties.SetPath("object_file", object)

command, err := b.prepareCommandForRecipe(properties, "recipe.ar.pattern", false)
if err != nil {
return errors.WithStack(err)
}

if err := b.execCommand(command); err != nil {
return errors.WithStack(err)
}
newArchives.AddIfMissing(archive)
}
for _, archive := range newArchives {
archiveDir := archive.Parent()
relatedObjectFiles := objectFiles.Clone()
relatedObjectFiles.Filter(func(object *paths.Path) bool {
// extract all the object files that are in the same directory of the archive
return object.Parent().EquivalentTo(archiveDir)
})
b.archiveCompiledFiles(archive, relatedObjectFiles)
}

objectFileList = strings.Join(f.Map(archives.AsStrings(), wrapWithDoubleQuotes), " ")
// Put everything together
allArchives := existingArchives.Clone()
allArchives.AddAll(newArchives)
objectFileList = strings.Join(f.Map(allArchives.AsStrings(), wrapWithDoubleQuotes), " ")
objectFileList = "-Wl,--whole-archive " + objectFileList + " -Wl,--no-whole-archive"
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ replace github.com/mailru/easyjson => github.com/cmaglie/easyjson v0.8.1

require (
github.com/ProtonMail/go-crypto v0.0.0-20230828082145-3c4c8a2d2371
github.com/arduino/go-paths-helper v1.9.2
github.com/arduino/go-paths-helper v1.11.0
github.com/arduino/go-properties-orderedmap v1.8.0
github.com/arduino/go-timeutils v0.0.0-20171220113728-d1dd9e313b1b
github.com/arduino/go-win32-utils v1.0.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ github.com/acomagu/bufpipe v1.0.3/go.mod h1:mxdxdup/WdsKVreO5GpW4+M/1CE2sMG4jeGJ
github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239 h1:kFOfPq6dUM1hTo4JG6LR5AXSUEsOjtdm0kw0FtQtMJA=
github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239/go.mod h1:2FmKhYUyUczH0OGQWaF5ceTx0UBShxjsH6f8oGKYe2c=
github.com/arduino/go-paths-helper v1.0.1/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck=
github.com/arduino/go-paths-helper v1.9.2 h1:omR8DPTL4nbUCWfGey5D+e3WvWfA2zEgoM6ZBRNt7ls=
github.com/arduino/go-paths-helper v1.9.2/go.mod h1:V82BWgAAp4IbmlybxQdk9Bpkz8M4Qyx+RAFKaG9NuvU=
github.com/arduino/go-paths-helper v1.11.0 h1:hkpGb9AtCTByTj2FKutuHWb3klDf4kAKL10hW+fN+oE=
github.com/arduino/go-paths-helper v1.11.0/go.mod h1:jcpW4wr0u69GlXhTYydsdsqAjLaYK5n7oWHfKqOG6LM=
github.com/arduino/go-properties-orderedmap v1.8.0 h1:wEfa6hHdpezrVOh787OmClsf/Kd8qB+zE3P2Xbrn0CQ=
github.com/arduino/go-properties-orderedmap v1.8.0/go.mod h1:DKjD2VXY/NZmlingh4lSFMEYCVubfeArCsGPGDwb2yk=
github.com/arduino/go-timeutils v0.0.0-20171220113728-d1dd9e313b1b h1:9hDi4F2st6dbLC3y4i02zFT5quS4X6iioWifGlVwfy4=
Expand Down
14 changes: 12 additions & 2 deletions internal/integrationtest/compile_1/compile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"encoding/json"
"fmt"
"os"
"regexp"
"sort"
"strings"
"testing"
Expand Down Expand Up @@ -830,8 +831,17 @@ func TestCompileWithArchivesAndLongPaths(t *testing.T) {
sketchPath := paths.New(libOutput[0]["library"].(map[string]interface{})["install_dir"].(string))
sketchPath = sketchPath.Join("examples", "ArduinoIoTCloud-Advanced")

_, _, err = cli.Run("compile", "-b", "esp8266:esp8266:huzzah", sketchPath.String(), "--config-file", "arduino-cli.yaml")
require.NoError(t, err)
t.Run("Compile", func(t *testing.T) {
_, _, err = cli.Run("compile", "-b", "esp8266:esp8266:huzzah", sketchPath.String(), "--config-file", "arduino-cli.yaml")
require.NoError(t, err)
})

t.Run("CheckCachingOfFolderArchives", func(t *testing.T) {
// Run compile again and check if the archive is re-used (cached)
out, _, err := cli.Run("compile", "-b", "esp8266:esp8266:huzzah", sketchPath.String(), "--config-file", "arduino-cli.yaml", "-v")
require.NoError(t, err)
require.True(t, regexp.MustCompile(`(?m)^Using previously compiled file:.*libraries.ArduinoIoTCloud.objs\.a$`).Match(out))
})
}

func TestCompileWithPrecompileLibrary(t *testing.T) {
Expand Down

0 comments on commit c2678cf

Please sign in to comment.