[ 
https://issues.apache.org/jira/browse/BEAM-3324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16293050#comment-16293050
 ] 

ASF GitHub Bot commented on BEAM-3324:
--------------------------------------

kennknowles closed pull request #4256: [BEAM-3324] Cache Go runtime symbol 
lookups
URL: https://github.com/apache/beam/pull/4256
 
 
   

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/runtime/symbols.go 
b/sdks/go/pkg/beam/core/runtime/symbols.go
index f9c839c948b..2659f4834eb 100644
--- a/sdks/go/pkg/beam/core/runtime/symbols.go
+++ b/sdks/go/pkg/beam/core/runtime/symbols.go
@@ -17,6 +17,7 @@ package runtime
 
 import (
        "os"
+       "sync"
 
        "github.com/apache/beam/sdks/go/pkg/beam/core/util/symtab"
 )
@@ -26,6 +27,7 @@ import (
 // SymbolResolution is the interface that should be implemented
 // in order to override the default behavior for symbol resolution.
 type SymbolResolution interface {
+       // Sym2Addr returns the address pointer for a given symbol.
        Sym2Addr(string) (uintptr, error)
 }
 
@@ -37,21 +39,49 @@ type SymbolResolution interface {
 var SymbolResolver SymbolResolution
 
 func init() {
-       var err error
        // First try the Linux location, since it's the most reliable.
-       SymbolResolver, err = symtab.New("/proc/self/exe")
-       if err == nil {
+       if r, err := symtab.New("/proc/self/exe"); err == nil {
+               SymbolResolver = NewSymbolCache(r)
                return
        }
        // For other OS's this works in most cases we need. If it doesn't, log
        // an error and keep going.
-       SymbolResolver, err = symtab.New(os.Args[0])
-       if err == nil {
+       if r, err := symtab.New(os.Args[0]); err == nil {
+               SymbolResolver = NewSymbolCache(r)
                return
        }
        SymbolResolver = panicResolver(false)
 }
 
+// SymbolCache added a caching layer over any SymbolResolution.
+type SymbolCache struct {
+       resolver SymbolResolution
+       cache    map[string]uintptr
+       mu       sync.Mutex
+}
+
+// NewSymbolCache adds caching to the given symbol resolver.
+func NewSymbolCache(r SymbolResolution) SymbolResolution {
+       return &SymbolCache{resolver: r, cache: make(map[string]uintptr)}
+}
+
+func (c *SymbolCache) Sym2Addr(sym string) (uintptr, error) {
+       c.mu.Lock()
+       defer c.mu.Unlock()
+
+       if p, exists := c.cache[sym]; exists {
+               return p, nil
+       }
+
+       p, err := c.resolver.Sym2Addr(sym)
+       if err != nil {
+               return 0, err
+       }
+
+       c.cache[sym] = p
+       return p, nil
+}
+
 type panicResolver bool
 
 func (p panicResolver) Sym2Addr(name string) (uintptr, error) {


 

----------------------------------------------------------------
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)

Reply via email to