[
https://issues.apache.org/jira/browse/BEAM-3324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16301702#comment-16301702
]
ASF GitHub Bot commented on BEAM-3324:
--------------------------------------
lukecwik closed pull request #4269: BEAM-3324 improve symtab memory usage
URL: https://github.com/apache/beam/pull/4269
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/sdks/go/pkg/beam/core/util/symtab/symtab.go
b/sdks/go/pkg/beam/core/util/symtab/symtab.go
index 16063171c98..0c631c48eeb 100644
--- a/sdks/go/pkg/beam/core/util/symtab/symtab.go
+++ b/sdks/go/pkg/beam/core/util/symtab/symtab.go
@@ -16,13 +16,12 @@
package symtab
import (
- "bytes"
"debug/dwarf"
"debug/elf"
"debug/macho"
"debug/pe"
"fmt"
- "io/ioutil"
+ "os"
)
// SymbolTable allows for mapping between symbols and their addresses.
@@ -33,43 +32,50 @@ type SymbolTable struct {
// New creates a new symbol table based on the debug info
// read from the specified file.
func New(filename string) (*SymbolTable, error) {
- b, err := ioutil.ReadFile(filename)
+ f, err := os.Open(filename)
if err != nil {
return nil, err
}
- r := bytes.NewReader(b)
+
+ // The interface contract for the xxx.NewFile() methods takes an
+ // io.ReaderAt which suggests the Reader needs to stay alive for the
duration
+ // of the symbol table.
// First try ELF
- ef, err := elf.NewFile(r)
+ ef, err := elf.NewFile(f)
if err == nil {
d, err := ef.DWARF()
if err != nil {
- return nil, err
+ f.Close()
+ return nil, fmt.Errorf("No working DWARF: %v", err)
}
return &SymbolTable{d}, nil
}
// then Mach-O
- mf, err := macho.NewFile(r)
+ mf, err := macho.NewFile(f)
if err == nil {
d, err := mf.DWARF()
if err != nil {
+ f.Close()
return nil, fmt.Errorf("No working DWARF: %v", err)
}
return &SymbolTable{d}, nil
}
// finally try Windows PE format
- pf, err := pe.NewFile(r)
+ pf, err := pe.NewFile(f)
if err == nil {
d, err := pf.DWARF()
if err != nil {
- return nil, err
+ f.Close()
+ return nil, fmt.Errorf("No working DWARF: %v", err)
}
return &SymbolTable{d}, nil
}
// Give up, we don't recognize it
+ f.Close()
return nil, fmt.Errorf("Unknown file format")
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> symtab.go shouldn't read entire file into memory
> ------------------------------------------------
>
> Key: BEAM-3324
> URL: https://issues.apache.org/jira/browse/BEAM-3324
> Project: Beam
> Issue Type: Improvement
> Components: sdk-go
> Reporter: Bill Neubauer
> Assignee: Bill Neubauer
> Priority: Minor
>
> The implementation of symtab.go reads the entire binary into memory. This is
> wasteful of memory, and it should just use os.File as the backing reader. If
> performance becomes an issue, we can use a modest amount of memory to cache
> lookups and avoid filesystem reads.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
