Skip to content

Commit

Permalink
cmd/link: fix link time regression in object file reading
Browse files Browse the repository at this point in the history 
In CL 173938, the linker's object file reader was switched over to
selectively create strings backed with read-only mmap'd memory.
In the process a call to r.rd.Offset() was added to readSymName(),
which greatly increased the number of system calls (Offset does a
seek system call).

This patch changes the object file reader so that all reads are done
directly from the mmap'd data if it is present, and adds logic to keep
track of the offset within the rodata consumed so far. Doing this gets
rid of the calls to r.rd.Offset() and the corresponding seek system
calls.

Also as part of this change, hoist the calls to objabi.PathToPrefix
up into the initial setup code for object reading, and store the
result in the reader (since objabi.PathToPrefix was also coming up
as hot in the profile).

Numbers for this change from compilebench:

benchmark                 old ns/op       new ns/op       delta
BenchmarkTemplate         172053975       170357597       -0.99%
BenchmarkUnicode          64564850        64333653        -0.36%
BenchmarkGoTypes          627931042       628043673       +0.02%
BenchmarkCompiler         2982468893      2924575043      -1.94%
BenchmarkSSA              9701681721      9799342557      +1.01%
BenchmarkFlate            106847240       107509414       +0.62%
BenchmarkGoParser         132082319       130734905       -1.02%
BenchmarkReflect          386810586       383036621       -0.98%
BenchmarkTar              154360072       152670594       -1.09%
BenchmarkXML              217725693       216858727       -0.40%
BenchmarkLinkCompiler     908813802       734363234       -19.20%
BenchmarkStdCmd           32378532486     31222542974     -3.57%

Fixes #31898.

Change-Id: Ibf253a52ce9213325f42b1c2b20d0410f5c88c3b
Reviewed-on: https://go-review.googlesource.com/c/go/+/176039
Reviewed-by: Cherry Zhang <cherryyz@google.com>
  • Loading branch information
@thanm
thanm committed May 9, 2019
1 parent fb63ed2 commit 0793c81
Showing 1 changed file with 46 additions and 22 deletions.
68 changes: 46 additions & 22 deletions src/cmd/link/internal/objfile/objfile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,10 @@ type objReader struct {
funcdata []*sym.Symbol
funcdataoff []int64
file []*sym.Symbol
pkgpref string // objabi.PathToPrefix(r.lib.Pkg) + "."

roObject []byte // from read-only mmap of object file
objFileOffset int64 // offset of object data from start of file
roObject []byte // from read-only mmap of object file (may be nil)
roOffset int64 // offset into readonly object data examined so far

dataReadOnly bool // whether data is backed by read-only memory
}
Expand Down Expand Up @@ -96,10 +97,15 @@ func Load(arch *sys.Arch, syms *sym.Symbols, f *bio.Reader, lib *sym.Library, le
localSymVersion: syms.IncVersion(),
flags: flags,
roObject: roObject,
objFileOffset: start,
pkgpref: objabi.PathToPrefix(lib.Pkg) + ".",
}
r.loadObjFile()
if f.Offset() != start+length {
if roObject != nil {
if r.roOffset != length {
log.Fatalf("%s: unexpected end at %d, want %d", pn, r.roOffset, start+length)
}
r.rd.Seek(int64(length), os.SEEK_CUR)
} else if f.Offset() != start+length {
log.Fatalf("%s: unexpected end at %d, want %d", pn, f.Offset(), start+length)
}
return r.strictDupMsgs
Expand All @@ -114,7 +120,7 @@ func (r *objReader) loadObjFile() {
}

// Version
c, err := r.rd.ReadByte()
c, err := r.readByte()
if err != nil || c != 1 {
log.Fatalf("%s: invalid file version number %d", r.pn, c)
}
Expand All @@ -131,12 +137,12 @@ func (r *objReader) loadObjFile() {
// Symbol references
r.refs = []*sym.Symbol{nil} // zeroth ref is nil
for {
c, err := r.rd.Peek(1)
c, err := r.peek(1)
if err != nil {
log.Fatalf("%s: peeking: %v", r.pn, err)
}
if c[0] == 0xff {
r.rd.ReadByte()
r.readByte()
break
}
r.readRef()
Expand All @@ -153,7 +159,7 @@ func (r *objReader) loadObjFile() {

// Defined symbols
for {
c, err := r.rd.Peek(1)
c, err := r.peek(1)
if err != nil {
log.Fatalf("%s: peeking: %v", r.pn, err)
}
Expand Down Expand Up @@ -188,10 +194,9 @@ func (r *objReader) readSlices() {

func (r *objReader) readDataSection() (err error) {
if r.roObject != nil {
dOffset := r.rd.Offset() - r.objFileOffset
r.data, r.dataReadOnly, err =
r.roObject[dOffset:dOffset+int64(r.dataSize)], true, nil
r.rd.Seek(int64(r.dataSize), os.SEEK_CUR)
r.roObject[r.roOffset:r.roOffset+int64(r.dataSize)], true, nil
r.roOffset += int64(r.dataSize)
return
}
r.data, r.dataReadOnly, err = r.rd.Slice(uint64(r.dataSize))
Expand All @@ -204,10 +209,10 @@ const symPrefix = 0xfe
func (r *objReader) readSym() {
var c byte
var err error
if c, err = r.rd.ReadByte(); c != symPrefix || err != nil {
if c, err = r.readByte(); c != symPrefix || err != nil {
log.Fatalln("readSym out of sync")
}
if c, err = r.rd.ReadByte(); err != nil {
if c, err = r.readByte(); err != nil {
log.Fatalln("error reading input: ", err)
}
t := sym.AbiSymKindToSymKind[c]
Expand All @@ -220,7 +225,6 @@ func (r *objReader) readSym() {
typ := r.readSymIndex()
data := r.readData()
nreloc := r.readInt()
pkg := objabi.PathToPrefix(r.lib.Pkg)
isdup := false

var dup *sym.Symbol
Expand Down Expand Up @@ -249,7 +253,7 @@ func (r *objReader) readSym() {
}

overwrite:
s.File = pkg
s.File = r.pkgpref[:len(r.pkgpref)-1]
s.Lib = r.lib
if dupok {
s.Attr |= sym.AttrDuplicateOK
Expand Down Expand Up @@ -432,7 +436,7 @@ func (r *objReader) patchDWARFName(s *sym.Symbol) {
if p == -1 {
return
}
pkgprefix := []byte(objabi.PathToPrefix(r.lib.Pkg) + ".")
pkgprefix := []byte(r.pkgpref)
patched := bytes.Replace(s.P[:e], emptyPkg, pkgprefix, -1)

s.P = append(patched, s.P[e:]...)
Expand All @@ -447,14 +451,35 @@ func (r *objReader) patchDWARFName(s *sym.Symbol) {
}

func (r *objReader) readFull(b []byte) {
if r.roObject != nil {
copy(b, r.roObject[r.roOffset:])
r.roOffset += int64(len(b))
return
}
_, err := io.ReadFull(r.rd, b)
if err != nil {
log.Fatalf("%s: error reading %s", r.pn, err)
}
}

func (r *objReader) readByte() (byte, error) {
if r.roObject != nil {
b := r.roObject[r.roOffset]
r.roOffset++
return b, nil
}
return r.rd.ReadByte()
}

func (r *objReader) peek(n int) ([]byte, error) {
if r.roObject != nil {
return r.roObject[r.roOffset : r.roOffset+int64(n)], nil
}
return r.rd.Peek(n)
}

func (r *objReader) readRef() {
if c, err := r.rd.ReadByte(); c != symPrefix || err != nil {
if c, err := r.readByte(); c != symPrefix || err != nil {
log.Fatalf("readSym out of sync")
}
name := r.readSymName()
Expand Down Expand Up @@ -505,7 +530,7 @@ func (r *objReader) readInt64() int64 {
if shift >= 64 {
log.Fatalf("corrupt input")
}
c, err := r.rd.ReadByte()
c, err := r.readByte()
if err != nil {
log.Fatalln("error reading input: ", err)
}
Expand Down Expand Up @@ -582,7 +607,6 @@ func mkROString(rodata []byte) string {

// readSymName reads a symbol name, replacing all "". with pkg.
func (r *objReader) readSymName() string {
pkg := objabi.PathToPrefix(r.lib.Pkg)
n := r.readInt()
if n == 0 {
r.readInt64()
Expand All @@ -591,8 +615,8 @@ func (r *objReader) readSymName() string {
if cap(r.rdBuf) < n {
r.rdBuf = make([]byte, 2*n)
}
sOffset := r.rd.Offset() - r.objFileOffset
origName, err := r.rd.Peek(n)
sOffset := r.roOffset
origName, err := r.peek(n)
if err == bufio.ErrBufferFull {
// Long symbol names are rare but exist. One source is type
// symbols for types with long string forms. See #15104.
Expand Down Expand Up @@ -623,7 +647,7 @@ func (r *objReader) readSymName() string {
}
nPkgRefs++
adjName = append(adjName, origName[:i]...)
adjName = append(adjName, pkg...)
adjName = append(adjName, r.pkgpref[:len(r.pkgpref)-1]...)
adjName = append(adjName, '.')
origName = origName[i+len(emptyPkg):]
}
Expand Down

0 comments on commit 0793c81

Please sign in to comment.