From 0802e274b281a33c4c8e99fe1f95f6a55513f9a5 Mon Sep 17 00:00:00 2001 From: Martino Facchin Date: Tue, 8 Nov 2016 15:31:47 +0100 Subject: [PATCH 1/5] Move size calculation from Java IDE --- src/arduino.cc/builder/builder.go | 2 + src/arduino.cc/builder/constants/constants.go | 14 ++ src/arduino.cc/builder/phases/sizer.go | 188 ++++++++++++++++++ 3 files changed, 204 insertions(+) create mode 100644 src/arduino.cc/builder/phases/sizer.go diff --git a/src/arduino.cc/builder/builder.go b/src/arduino.cc/builder/builder.go index c3202521..759ce84e 100644 --- a/src/arduino.cc/builder/builder.go +++ b/src/arduino.cc/builder/builder.go @@ -116,6 +116,8 @@ func (s *Builder) Run(ctx *types.Context) error { &RecipeByPrefixSuffixRunner{Prefix: "recipe.objcopy.", Suffix: constants.HOOKS_PATTERN_SUFFIX}, &RecipeByPrefixSuffixRunner{Prefix: constants.HOOKS_OBJCOPY_POSTOBJCOPY, Suffix: constants.HOOKS_PATTERN_SUFFIX}, + &phases.Sizer{}, + &MergeSketchWithBootloader{}, &RecipeByPrefixSuffixRunner{Prefix: constants.HOOKS_POSTBUILD, Suffix: constants.HOOKS_PATTERN_SUFFIX}, diff --git a/src/arduino.cc/builder/constants/constants.go b/src/arduino.cc/builder/constants/constants.go index 62147d62..cdbdb72b 100644 --- a/src/arduino.cc/builder/constants/constants.go +++ b/src/arduino.cc/builder/constants/constants.go @@ -170,6 +170,13 @@ const MSG_PROP_IN_LIBRARY = "Missing '{0}' from library in {1}" const MSG_RUNNING_COMMAND = "Ts: {0} - Running: {1}" const MSG_RUNNING_RECIPE = "Running recipe: {0}" const MSG_SETTING_BUILD_PATH = "Setting build path to {0}" +const MSG_SIZER_TEXT_FULL = "Sketch uses {0} bytes ({2}%%) of program storage space. Maximum is {1} bytes." +const MSG_SIZER_DATA_FULL = "Global variables use {0} bytes ({2}%%) of dynamic memory, leaving {3} bytes for local variables. Maximum is {1} bytes." +const MSG_SIZER_DATA = "Global variables use {0} bytes of dynamic memory." +const MSG_SIZER_TEXT_TOO_BIG = "Sketch too big; see http://www.arduino.cc/en/Guide/Troubleshooting#size for tips on reducing it." +const MSG_SIZER_DATA_TOO_BIG = "Not enough memory; see http://www.arduino.cc/en/Guide/Troubleshooting#size for tips on reducing your footprint." +const MSG_SIZER_LOW_MEMORY = "Low memory available, stability problems may occur." +const MSG_SIZER_ERROR_NO_RULE = "Couldn't determine program size" const MSG_SKETCH_CANT_BE_IN_BUILDPATH = "Sketch cannot be located in build path. Please specify a different build path" const MSG_SKIPPING_TAG_ALREADY_DEFINED = "Skipping tag {0} because prototype is already defined" const MSG_SKIPPING_TAG_BECAUSE_HAS_FIELD = "Skipping tag {0} because it has field {0}" @@ -197,14 +204,21 @@ const PLATFORM_REWRITE_NEW = "new" const PLATFORM_REWRITE_OLD = "old" const PLATFORM_URL = "url" const PLATFORM_VERSION = "version" +const PROPERTY_WARN_DATA_PERCENT = "build.warn_data_percentage" +const PROPERTY_UPLOAD_MAX_SIZE = "upload.maximum_size" +const PROPERTY_UPLOAD_MAX_DATA_SIZE = "upload.maximum_data_size" const PROGRAMMER_NAME = "name" const RECIPE_AR_PATTERN = "recipe.ar.pattern" const RECIPE_C_COMBINE_PATTERN = "recipe.c.combine.pattern" const RECIPE_C_PATTERN = "recipe.c.o.pattern" const RECIPE_CPP_PATTERN = "recipe.cpp.o.pattern" +const RECIPE_SIZE_PATTERN = "recipe.size.pattern" const RECIPE_PREPROC_INCLUDES = "recipe.preproc.includes" const RECIPE_PREPROC_MACROS = "recipe.preproc.macros" const RECIPE_S_PATTERN = "recipe.S.o.pattern" +const RECIPE_SIZE_REGEXP = "recipe.size.regex" +const RECIPE_SIZE_REGEXP_DATA = "recipe.size.regex.data" +const RECIPE_SIZE_REGEXP_EEPROM = "recipe.size.regex.eeprom" const REWRITING_DISABLED = "disabled" const REWRITING = "rewriting" const SPACE = " " diff --git a/src/arduino.cc/builder/phases/sizer.go b/src/arduino.cc/builder/phases/sizer.go new file mode 100644 index 00000000..ab75def3 --- /dev/null +++ b/src/arduino.cc/builder/phases/sizer.go @@ -0,0 +1,188 @@ +/* + * This file is part of Arduino Builder. + * + * Arduino Builder is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + * As a special exception, you may use this file as part of a free software + * library without restriction. Specifically, if other files instantiate + * templates or use macros or inline functions from this file, or you compile + * this file and link it with other files to produce an executable, this + * file does not by itself cause the resulting executable to be covered by + * the GNU General Public License. This exception does not however + * invalidate any other reasons why the executable file might be covered by + * the GNU General Public License. + * + * Copyright 2016 Arduino LLC (http://www.arduino.cc/) + */ + +package phases + +import ( + "errors" + "regexp" + "strconv" + + "arduino.cc/builder/builder_utils" + "arduino.cc/builder/constants" + "arduino.cc/builder/i18n" + "arduino.cc/builder/types" + "arduino.cc/properties" +) + +type Sizer struct{} + +func (s *Sizer) Run(ctx *types.Context) error { + buildProperties := ctx.BuildProperties + verbose := ctx.Verbose + warningsLevel := ctx.WarningsLevel + logger := ctx.GetLogger() + + err := checkSize(buildProperties, verbose, warningsLevel, logger) + if err != nil { + return i18n.WrapError(err) + } + + return nil +} + +func checkSize(buildProperties properties.Map, verbose bool, warningsLevel string, logger i18n.Logger) error { + + properties := buildProperties.Clone() + properties[constants.BUILD_PROPERTIES_COMPILER_WARNING_FLAGS] = properties[constants.BUILD_PROPERTIES_COMPILER_WARNING_FLAGS+"."+warningsLevel] + + maxTextSizeString := properties[constants.PROPERTY_UPLOAD_MAX_SIZE] + maxDataSizeString := properties[constants.PROPERTY_UPLOAD_MAX_DATA_SIZE] + + if maxTextSizeString == "" { + return nil + } + + maxTextSize, err := strconv.Atoi(maxTextSizeString) + if err != nil { + return err + } + + maxDataSize := -1 + if maxDataSizeString != "" { + maxDataSize, err = strconv.Atoi(maxDataSizeString) + if err != nil { + return err + } + } + + textSize, dataSize, _, err := execSizeReceipe(properties, logger) + if err != nil { + logger.Println(constants.LOG_LEVEL_WARN, constants.MSG_SIZER_ERROR_NO_RULE) + return nil + } + + logger.Println(constants.LOG_LEVEL_INFO, constants.MSG_SIZER_TEXT_FULL, strconv.Itoa(textSize), strconv.Itoa(maxTextSize), strconv.Itoa(textSize*100/maxTextSize)) + if dataSize >= 0 { + if maxDataSize > 0 { + logger.Println(constants.LOG_LEVEL_INFO, constants.MSG_SIZER_DATA_FULL, strconv.Itoa(dataSize), strconv.Itoa(maxDataSize), strconv.Itoa(dataSize*100/maxDataSize), strconv.Itoa(maxDataSize-dataSize)) + } else { + logger.Println(constants.LOG_LEVEL_INFO, constants.MSG_SIZER_DATA, strconv.Itoa(dataSize)) + } + } + + if textSize > maxTextSize { + logger.Println(constants.LOG_LEVEL_ERROR, constants.MSG_SIZER_TEXT_TOO_BIG) + return errors.New("") + } + + if maxDataSize > 0 && dataSize > maxDataSize { + logger.Println(constants.LOG_LEVEL_ERROR, constants.MSG_SIZER_DATA_TOO_BIG) + return errors.New("") + } + + if properties[constants.PROPERTY_WARN_DATA_PERCENT] != "" { + warnDataPercentage, err := strconv.Atoi(properties[constants.PROPERTY_WARN_DATA_PERCENT]) + if err != nil { + return err + } + if maxDataSize > 0 && dataSize > maxDataSize*warnDataPercentage/100 { + logger.Println(constants.LOG_LEVEL_WARN, constants.MSG_SIZER_LOW_MEMORY) + } + } + + return nil +} + +func execSizeReceipe(properties properties.Map, logger i18n.Logger) (textSize int, dataSize int, eepromSize int, resErr error) { + out, err := builder_utils.ExecRecipe(properties, constants.RECIPE_SIZE_PATTERN, false, false, false, logger) + if err != nil { + resErr = errors.New("Error while determining sketch size: " + err.Error()) + return + } + + // force multiline match prepending "(?m)" to the actual regexp + // return an error if RECIPE_SIZE_REGEXP doesn't exist + + if len(properties[constants.RECIPE_SIZE_REGEXP]) > 0 { + textRegexp, err := regexp.Compile("(?m)" + properties[constants.RECIPE_SIZE_REGEXP]) + if err != nil { + resErr = errors.New("Invalid size regexp: " + err.Error()) + return + } + result := textRegexp.FindAllSubmatch(out, -1) + for _, b := range result { + for _, c := range b { + if res, err := strconv.Atoi(string(c)); err == nil { + textSize += res + } + } + } + } else { + resErr = errors.New("Missing size regexp") + return + } + + if len(properties[constants.RECIPE_SIZE_REGEXP_DATA]) > 0 { + dataRegexp, err := regexp.Compile("(?m)" + properties[constants.RECIPE_SIZE_REGEXP_DATA]) + if err != nil { + resErr = errors.New("Invalid data size regexp: " + err.Error()) + return + } + result := dataRegexp.FindAllSubmatch(out, -1) + for _, b := range result { + for _, c := range b { + if res, err := strconv.Atoi(string(c)); err == nil { + dataSize += res + } + } + } + } else { + dataSize = -1 + } + + if len(properties[constants.RECIPE_SIZE_REGEXP_EEPROM]) > 0 { + eepromRegexp, err := regexp.Compile("(?m)" + properties[constants.RECIPE_SIZE_REGEXP_EEPROM]) + if err != nil { + resErr = errors.New("Invalid eeprom size regexp: " + err.Error()) + return + } + result := eepromRegexp.FindAllSubmatch(out, -1) + for _, b := range result { + for _, c := range b { + if res, err := strconv.Atoi(string(c)); err == nil { + eepromSize += res + } + } + } + } else { + eepromSize = -1 + } + return +} From 34ae019248ebb62b82e00085a9f7fafe95ec91d3 Mon Sep 17 00:00:00 2001 From: Martino Facchin Date: Thu, 17 Nov 2016 12:43:20 +0100 Subject: [PATCH 2/5] Fix double indirection for %% literal Signed-off-by: Martino Facchin --- src/arduino.cc/builder/i18n/i18n.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/arduino.cc/builder/i18n/i18n.go b/src/arduino.cc/builder/i18n/i18n.go index 2769e587..fbc53a1c 100644 --- a/src/arduino.cc/builder/i18n/i18n.go +++ b/src/arduino.cc/builder/i18n/i18n.go @@ -65,7 +65,7 @@ func (s HumanLogger) Fprintln(w io.Writer, level string, format string, a ...int } func (s HumanLogger) Println(level string, format string, a ...interface{}) { - s.Fprintln(os.Stdout, level, Format(format, a...)) + s.Fprintln(os.Stdout, level, format, a...) } func (s HumanLogger) Name() string { From b60ac1c776cf01070c3be0f18973bff1e2ef8465 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 24 Nov 2016 14:07:02 +0100 Subject: [PATCH 3/5] Factored out size computation function. This helps creating the test on the next commit. Signed-off-by: Cristian Maglie --- src/arduino.cc/builder/phases/sizer.go | 80 +++++++++++--------------- 1 file changed, 34 insertions(+), 46 deletions(-) diff --git a/src/arduino.cc/builder/phases/sizer.go b/src/arduino.cc/builder/phases/sizer.go index ab75def3..5b57c432 100644 --- a/src/arduino.cc/builder/phases/sizer.go +++ b/src/arduino.cc/builder/phases/sizer.go @@ -130,59 +130,47 @@ func execSizeReceipe(properties properties.Map, logger i18n.Logger) (textSize in // force multiline match prepending "(?m)" to the actual regexp // return an error if RECIPE_SIZE_REGEXP doesn't exist - if len(properties[constants.RECIPE_SIZE_REGEXP]) > 0 { - textRegexp, err := regexp.Compile("(?m)" + properties[constants.RECIPE_SIZE_REGEXP]) - if err != nil { - resErr = errors.New("Invalid size regexp: " + err.Error()) - return - } - result := textRegexp.FindAllSubmatch(out, -1) - for _, b := range result { - for _, c := range b { - if res, err := strconv.Atoi(string(c)); err == nil { - textSize += res - } - } - } - } else { + textSize, err = computeSize(properties[constants.RECIPE_SIZE_REGEXP], out) + if err != nil { + resErr = errors.New("Invalid size regexp: " + err.Error()) + return + } + if textSize == -1 { resErr = errors.New("Missing size regexp") return } - if len(properties[constants.RECIPE_SIZE_REGEXP_DATA]) > 0 { - dataRegexp, err := regexp.Compile("(?m)" + properties[constants.RECIPE_SIZE_REGEXP_DATA]) - if err != nil { - resErr = errors.New("Invalid data size regexp: " + err.Error()) - return - } - result := dataRegexp.FindAllSubmatch(out, -1) - for _, b := range result { - for _, c := range b { - if res, err := strconv.Atoi(string(c)); err == nil { - dataSize += res - } - } - } - } else { - dataSize = -1 + dataSize, err = computeSize(properties[constants.RECIPE_SIZE_REGEXP_DATA], out) + if err != nil { + resErr = errors.New("Invalid data size regexp: " + err.Error()) + return } - if len(properties[constants.RECIPE_SIZE_REGEXP_EEPROM]) > 0 { - eepromRegexp, err := regexp.Compile("(?m)" + properties[constants.RECIPE_SIZE_REGEXP_EEPROM]) - if err != nil { - resErr = errors.New("Invalid eeprom size regexp: " + err.Error()) - return - } - result := eepromRegexp.FindAllSubmatch(out, -1) - for _, b := range result { - for _, c := range b { - if res, err := strconv.Atoi(string(c)); err == nil { - eepromSize += res - } + eepromSize, err = computeSize(properties[constants.RECIPE_SIZE_REGEXP_EEPROM], out) + if err != nil { + resErr = errors.New("Invalid eeprom size regexp: " + err.Error()) + return + } + + return +} + +func computeSize(re string, output []byte) (int, error) { + if re == "" { + return -1, nil + } + r, err := regexp.Compile("(?m)" + re) + if err != nil { + return -1, err + } + result := r.FindAllSubmatch(output, -1) + size := 0 + for _, b := range result { + for _, c := range b { + if res, err := strconv.Atoi(string(c)); err == nil { + size += res } } - } else { - eepromSize = -1 } - return + return size, nil } From 65aa1e6ec815c216029b5740b066c5171d94da7c Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 24 Nov 2016 14:08:03 +0100 Subject: [PATCH 4/5] Added tests for sizer Signed-off-by: Cristian Maglie --- src/arduino.cc/builder/phases/sizer_test.go | 102 ++++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 src/arduino.cc/builder/phases/sizer_test.go diff --git a/src/arduino.cc/builder/phases/sizer_test.go b/src/arduino.cc/builder/phases/sizer_test.go new file mode 100644 index 00000000..3f0f704f --- /dev/null +++ b/src/arduino.cc/builder/phases/sizer_test.go @@ -0,0 +1,102 @@ +/* + * This file is part of Arduino Builder. + * + * Arduino Builder is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + * As a special exception, you may use this file as part of a free software + * library without restriction. Specifically, if other files instantiate + * templates or use macros or inline functions from this file, or you compile + * this file and link it with other files to produce an executable, this + * file does not by itself cause the resulting executable to be covered by + * the GNU General Public License. This exception does not however + * invalidate any other reasons why the executable file might be covered by + * the GNU General Public License. + * + * Copyright 2016 Arduino LLC (http://www.arduino.cc/) + */ + +package phases + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSizerWithAVRData(t *testing.T) { + output := []byte(`/tmp/test597119152/sketch.ino.elf : +section size addr +.data 36 8388864 +.text 3966 0 +.bss 112 8388900 +.comment 17 0 +.debug_aranges 704 0 +.debug_info 21084 0 +.debug_abbrev 4704 0 +.debug_line 5456 0 +.debug_frame 1964 0 +.debug_str 8251 0 +.debug_loc 7747 0 +.debug_ranges 856 0 +Total 54897 +`) + + size, err := computeSize(`^(?:\.text|\.data|\.bootloader)\s+([0-9]+).*`, output) + require.NoError(t, err) + require.Equal(t, 4002, size) + + size, err = computeSize(`^(?:\.data|\.bss|\.noinit)\s+([0-9]+).*`, output) + require.NoError(t, err) + require.Equal(t, 148, size) + + size, err = computeSize(`^(?:\.eeprom)\s+([0-9]+).*`, output) + require.NoError(t, err) + require.Equal(t, 0, size) +} + +func TestSizerWithSAMDData(t *testing.T) { + output := []byte(`/tmp/test737785204/sketch_usbhost.ino.elf : +section size addr +.text 8076 8192 +.data 152 536870912 +.bss 1984 536871064 +.ARM.attributes 40 0 +.comment 128 0 +.debug_info 178841 0 +.debug_abbrev 14968 0 +.debug_aranges 2080 0 +.debug_ranges 3840 0 +.debug_line 26068 0 +.debug_str 55489 0 +.debug_frame 5036 0 +.debug_loc 20978 0 +Total 317680 +`) + + size, err := computeSize(`\.text\s+([0-9]+).*`, output) + require.NoError(t, err) + require.Equal(t, 8076, size) +} + +func TestSizerEmptyRegexpReturnsMinusOne(t *testing.T) { + size, err := computeSize(``, []byte(`xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx`)) + require.NoError(t, err) + require.Equal(t, -1, size) +} + +func TestSizerWithInvalidRegexp(t *testing.T) { + _, err := computeSize(`[xx`, []byte(`xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx`)) + require.Error(t, err) +} From 3a4c024580707d3cf633e86b2ee48791bb5ff0d0 Mon Sep 17 00:00:00 2001 From: Martino Facchin Date: Thu, 24 Nov 2016 16:50:25 +0100 Subject: [PATCH 5/5] Move sizer after PrintUsedLibraries Signed-off-by: Martino Facchin --- src/arduino.cc/builder/builder.go | 4 ++-- src/arduino.cc/builder/phases/sizer.go | 9 ++++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/arduino.cc/builder/builder.go b/src/arduino.cc/builder/builder.go index 759ce84e..6de04a40 100644 --- a/src/arduino.cc/builder/builder.go +++ b/src/arduino.cc/builder/builder.go @@ -116,8 +116,6 @@ func (s *Builder) Run(ctx *types.Context) error { &RecipeByPrefixSuffixRunner{Prefix: "recipe.objcopy.", Suffix: constants.HOOKS_PATTERN_SUFFIX}, &RecipeByPrefixSuffixRunner{Prefix: constants.HOOKS_OBJCOPY_POSTOBJCOPY, Suffix: constants.HOOKS_PATTERN_SUFFIX}, - &phases.Sizer{}, - &MergeSketchWithBootloader{}, &RecipeByPrefixSuffixRunner{Prefix: constants.HOOKS_POSTBUILD, Suffix: constants.HOOKS_PATTERN_SUFFIX}, @@ -129,6 +127,8 @@ func (s *Builder) Run(ctx *types.Context) error { &PrintUsedAndNotUsedLibraries{SketchError: mainErr != nil}, &PrintUsedLibrariesIfVerbose{}, + + &phases.Sizer{SketchError: mainErr != nil}, } otherErr := runCommands(ctx, commands, false) diff --git a/src/arduino.cc/builder/phases/sizer.go b/src/arduino.cc/builder/phases/sizer.go index 5b57c432..b4373a2d 100644 --- a/src/arduino.cc/builder/phases/sizer.go +++ b/src/arduino.cc/builder/phases/sizer.go @@ -41,9 +41,16 @@ import ( "arduino.cc/properties" ) -type Sizer struct{} +type Sizer struct { + SketchError bool +} func (s *Sizer) Run(ctx *types.Context) error { + + if s.SketchError { + return nil + } + buildProperties := ctx.BuildProperties verbose := ctx.Verbose warningsLevel := ctx.WarningsLevel