From: Jeremy Faller Date: Thu, 16 Apr 2020 23:33:11 +0000 (+0000) Subject: Revert "[dev.link] cmd/link: remove buffered file I/O from OutBuf" X-Git-Tag: go1.15beta1~401^2^2~23 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=7fe3f30bbbcb89ced4fb2cd4cbb93c8a0986a973;p=gostls13.git Revert "[dev.link] cmd/link: remove buffered file I/O from OutBuf" This reverts commit b2def42d9efcf4540656e26632b744f8e7436814. Reason for revert: trybots failing Change-Id: I920be6d8de158b1e513154ac0eb0c8fa0cffa9f4 Reviewed-on: https://go-review.googlesource.com/c/go/+/228657 Reviewed-by: Than McIntosh Reviewed-by: Cherry Zhang --- diff --git a/src/cmd/link/internal/amd64/asm.go b/src/cmd/link/internal/amd64/asm.go index d26a9a234c..5c4ffe19c2 100644 --- a/src/cmd/link/internal/amd64/asm.go +++ b/src/cmd/link/internal/amd64/asm.go @@ -756,6 +756,7 @@ func asmb2(ctxt *ld.Link) { if ctxt.IsELF { ctxt.Out.SeekSet(symo) ld.Asmelfsym(ctxt) + ctxt.Out.Flush() ctxt.Out.Write(ld.Elfstrdat) if ctxt.LinkMode == ld.LinkExternal { @@ -765,11 +766,13 @@ func asmb2(ctxt *ld.Link) { case objabi.Hplan9: ld.Asmplan9sym(ctxt) + ctxt.Out.Flush() sym := ctxt.Syms.Lookup("pclntab", 0) if sym != nil { ld.Lcsize = int32(len(sym.P)) ctxt.Out.Write(sym.P) + ctxt.Out.Flush() } case objabi.Hwindows: @@ -814,6 +817,8 @@ func asmb2(ctxt *ld.Link) { case objabi.Hwindows: ld.Asmbpe(ctxt) } + + ctxt.Out.Flush() } func tlsIEtoLE(s *sym.Symbol, off, size int) { diff --git a/src/cmd/link/internal/arm/asm.go b/src/cmd/link/internal/arm/asm.go index e9eea5ce2c..f3d1262879 100644 --- a/src/cmd/link/internal/arm/asm.go +++ b/src/cmd/link/internal/arm/asm.go @@ -816,6 +816,7 @@ func asmb2(ctxt *ld.Link) { default: if ctxt.IsELF { ld.Asmelfsym(ctxt) + ctxt.Out.Flush() ctxt.Out.Write(ld.Elfstrdat) if ctxt.LinkMode == ld.LinkExternal { @@ -825,11 +826,13 @@ func asmb2(ctxt *ld.Link) { case objabi.Hplan9: ld.Asmplan9sym(ctxt) + ctxt.Out.Flush() sym := ctxt.Syms.Lookup("pclntab", 0) if sym != nil { ld.Lcsize = int32(len(sym.P)) ctxt.Out.Write(sym.P) + ctxt.Out.Flush() } case objabi.Hwindows: @@ -860,6 +863,7 @@ func asmb2(ctxt *ld.Link) { ld.Asmbpe(ctxt) } + ctxt.Out.Flush() if *ld.FlagC { fmt.Printf("textsize=%d\n", ld.Segtext.Filelen) fmt.Printf("datsize=%d\n", ld.Segdata.Filelen) diff --git a/src/cmd/link/internal/arm64/asm.go b/src/cmd/link/internal/arm64/asm.go index 46bda74c4c..053b8d119d 100644 --- a/src/cmd/link/internal/arm64/asm.go +++ b/src/cmd/link/internal/arm64/asm.go @@ -866,6 +866,7 @@ func asmb2(ctxt *ld.Link) { default: if ctxt.IsELF { ld.Asmelfsym(ctxt) + ctxt.Out.Flush() ctxt.Out.Write(ld.Elfstrdat) if ctxt.LinkMode == ld.LinkExternal { @@ -875,11 +876,13 @@ func asmb2(ctxt *ld.Link) { case objabi.Hplan9: ld.Asmplan9sym(ctxt) + ctxt.Out.Flush() sym := ctxt.Syms.Lookup("pclntab", 0) if sym != nil { ld.Lcsize = int32(len(sym.P)) ctxt.Out.Write(sym.P) + ctxt.Out.Flush() } case objabi.Hdarwin: @@ -912,6 +915,7 @@ func asmb2(ctxt *ld.Link) { ld.Asmbmacho(ctxt) } + ctxt.Out.Flush() if *ld.FlagC { fmt.Printf("textsize=%d\n", ld.Segtext.Filelen) fmt.Printf("datsize=%d\n", ld.Segdata.Filelen) diff --git a/src/cmd/link/internal/ld/data.go b/src/cmd/link/internal/ld/data.go index 36708ee5d1..3979880cf4 100644 --- a/src/cmd/link/internal/ld/data.go +++ b/src/cmd/link/internal/ld/data.go @@ -32,6 +32,7 @@ package ld import ( + "bufio" "bytes" "cmd/internal/gcprog" "cmd/internal/objabi" @@ -957,10 +958,11 @@ func Datblk(ctxt *Link, out *OutBuf, addr, size int64) { // Used only on Wasm for now. func DatblkBytes(ctxt *Link, addr int64, size int64) []byte { - buf := make([]byte, size) - out := &OutBuf{heap: buf} + buf := bytes.NewBuffer(make([]byte, 0, size)) + out := &OutBuf{w: bufio.NewWriter(buf)} writeDatblkToOutBuf(ctxt, out, addr, size) - return buf + out.Flush() + return buf.Bytes() } func writeDatblkToOutBuf(ctxt *Link, out *OutBuf, addr int64, size int64) { diff --git a/src/cmd/link/internal/ld/main.go b/src/cmd/link/internal/ld/main.go index ff9d1b51a3..dd089e6efa 100644 --- a/src/cmd/link/internal/ld/main.go +++ b/src/cmd/link/internal/ld/main.go @@ -319,25 +319,35 @@ func Main(arch *sys.Arch, theArch Arch) { // for which we have computed the size and offset, in a // mmap'd region. The second part writes more content, for // which we don't know the size. + var outputMmapped bool if ctxt.Arch.Family != sys.Wasm { // Don't mmap if we're building for Wasm. Wasm file // layout is very different so filesize is meaningless. - if err := ctxt.Out.Mmap(filesize); err != nil { - ctxt.Errorf(0, "error mapping file: %v", err) - } + err := ctxt.Out.Mmap(filesize) + outputMmapped = err == nil + } + if outputMmapped { + // Asmb will redirect symbols to the output file mmap, and relocations + // will be applied directly there. + bench.Start("Asmb") + thearch.Asmb(ctxt) + bench.Start("reloc") + ctxt.reloc() + } else { + // If we don't mmap, we need to apply relocations before + // writing out. + bench.Start("reloc") + ctxt.reloc() + bench.Start("Asmb") + thearch.Asmb(ctxt) } - - // Asmb will redirect symbols to the output file mmap, and relocations - // will be applied directly there. - bench.Start("Asmb") - thearch.Asmb(ctxt) - bench.Start("reloc") - ctxt.reloc() bench.Start("Asmb2") thearch.Asmb2(ctxt) - bench.Start("Munmap") - ctxt.Out.Close() // Close handles Munmapping if necessary. + if outputMmapped { + bench.Start("Munmap") + ctxt.Out.Munmap() + } bench.Start("undef") ctxt.undef() diff --git a/src/cmd/link/internal/ld/outbuf.go b/src/cmd/link/internal/ld/outbuf.go index 490d9b5b7a..c36fc74a44 100644 --- a/src/cmd/link/internal/ld/outbuf.go +++ b/src/cmd/link/internal/ld/outbuf.go @@ -5,6 +5,7 @@ package ld import ( + "bufio" "cmd/internal/sys" "cmd/link/internal/sym" "encoding/binary" @@ -60,6 +61,7 @@ type OutBuf struct { buf []byte // backing store of mmap'd output file heap []byte // backing store for non-mmapped data + w *bufio.Writer name string f *os.File encbuf [8]byte // temp buffer used by WriteN methods @@ -76,6 +78,7 @@ func (out *OutBuf) Open(name string) error { } out.off = 0 out.name = name + out.w = bufio.NewWriter(f) out.f = f return nil } @@ -107,17 +110,10 @@ func (out *OutBuf) Close() error { if out.isView { return viewCloseError } - if out.isMmapped() { - return out.Munmap() - } + out.Flush() if out.f == nil { return nil } - if len(out.heap) != 0 { - if _, err := out.f.Write(out.heap); err != nil { - return err - } - } if err := out.f.Close(); err != nil { return err } @@ -155,6 +151,10 @@ func (out *OutBuf) Munmap() error { // writing. When the mmapped section is full, we switch over the heap memory // for writing. func (out *OutBuf) writeLoc(lenToWrite int64) (int64, []byte) { + if !out.isMmapped() { + panic("shouldn't happen") + } + // See if we have enough space in the mmaped area. bufLen := int64(len(out.buf)) if out.off+lenToWrite <= bufLen { @@ -178,6 +178,15 @@ func (out *OutBuf) writeLoc(lenToWrite int64) (int64, []byte) { } func (out *OutBuf) SeekSet(p int64) { + if p == out.off { + return + } + if !out.isMmapped() { + out.Flush() + if _, err := out.f.Seek(p, 0); err != nil { + Exitf("seeking to %d in %s: %v", p, out.name, err) + } + } out.off = p } @@ -186,18 +195,33 @@ func (out *OutBuf) Offset() int64 { } // Write writes the contents of v to the buffer. +// +// As Write is backed by a bufio.Writer, callers do not have +// to explicitly handle the returned error as long as Flush is +// eventually called. func (out *OutBuf) Write(v []byte) (int, error) { - n := len(v) - pos, buf := out.writeLoc(int64(n)) - copy(buf[pos:], v) + if out.isMmapped() { + n := len(v) + pos, buf := out.writeLoc(int64(n)) + copy(buf[pos:], v) + out.off += int64(n) + return n, nil + } + n, err := out.w.Write(v) out.off += int64(n) - return n, nil + return n, err } func (out *OutBuf) Write8(v uint8) { - pos, buf := out.writeLoc(1) - buf[pos] = v - out.off++ + if out.isMmapped() { + pos, buf := out.writeLoc(1) + buf[pos] = v + out.off++ + return + } + if err := out.w.WriteByte(v); err == nil { + out.off++ + } } // WriteByte is an alias for Write8 to fulfill the io.ByteWriter interface. @@ -232,11 +256,16 @@ func (out *OutBuf) Write64b(v uint64) { } func (out *OutBuf) WriteString(s string) { - pos, buf := out.writeLoc(int64(len(s))) - n := copy(buf[pos:], s) - if n != len(s) { - log.Fatalf("WriteString truncated. buffer size: %d, offset: %d, len(s)=%d", len(out.buf), out.off, len(s)) + if out.isMmapped() { + pos, buf := out.writeLoc(int64(len(s))) + n := copy(buf[pos:], s) + if n != len(s) { + log.Fatalf("WriteString truncated. buffer size: %d, offset: %d, len(s)=%d", len(out.buf), out.off, len(s)) + } + out.off += int64(n) + return } + n, _ := out.w.WriteString(s) out.off += int64(n) } @@ -268,10 +297,26 @@ func (out *OutBuf) WriteStringPad(s string, n int, pad []byte) { // edit to the symbol content. // If the output file is not Mmap'd, just writes the content. func (out *OutBuf) WriteSym(s *sym.Symbol) { - n := int64(len(s.P)) - pos, buf := out.writeLoc(n) - copy(buf[pos:], s.P) - out.off += n - s.P = buf[pos : pos+n] - s.Attr.Set(sym.AttrReadOnly, false) + // NB: We inline the Write call for speediness. + if out.isMmapped() { + n := int64(len(s.P)) + pos, buf := out.writeLoc(n) + copy(buf[pos:], s.P) + out.off += n + s.P = buf[pos : pos+n] + s.Attr.Set(sym.AttrReadOnly, false) + } else { + n, _ := out.w.Write(s.P) + out.off += int64(n) + } +} + +func (out *OutBuf) Flush() { + var err error + if out.w != nil { + err = out.w.Flush() + } + if err != nil { + Exitf("flushing %s: %v", out.name, err) + } } diff --git a/src/cmd/link/internal/ld/outbuf_test.go b/src/cmd/link/internal/ld/outbuf_test.go index 58f9b10cfa..aae206f511 100644 --- a/src/cmd/link/internal/ld/outbuf_test.go +++ b/src/cmd/link/internal/ld/outbuf_test.go @@ -50,7 +50,6 @@ func TestWriteLoc(t *testing.T) { {100, 100, 0, 100, 100, 0, true}, {10, 10, 0, 100, 100, 0, true}, {10, 20, 10, 100, 110, 10, true}, - {0, 0, 0, 100, 100, 0, true}, } for i, test := range tests { diff --git a/src/cmd/link/internal/ld/xcoff.go b/src/cmd/link/internal/ld/xcoff.go index 9fe3669eee..4ff123e8cd 100644 --- a/src/cmd/link/internal/ld/xcoff.go +++ b/src/cmd/link/internal/ld/xcoff.go @@ -1389,6 +1389,7 @@ func (f *xcoffFile) writeLdrScn(ctxt *Link, globalOff uint64) { } f.loaderSize = off + uint64(stlen) + ctxt.Out.Flush() /* again for printing */ if !*flagA { @@ -1558,6 +1559,8 @@ func Asmbxcoff(ctxt *Link, fileoff int64) { // write string table xfile.stringTable.write(ctxt.Out) + ctxt.Out.Flush() + // write headers xcoffwrite(ctxt) } diff --git a/src/cmd/link/internal/mips/asm.go b/src/cmd/link/internal/mips/asm.go index 21a57ccbb0..b50112ad75 100644 --- a/src/cmd/link/internal/mips/asm.go +++ b/src/cmd/link/internal/mips/asm.go @@ -204,6 +204,7 @@ func asmb2(ctxt *ld.Link) { ctxt.Out.SeekSet(int64(symo)) ld.Asmelfsym(ctxt) + ctxt.Out.Flush() ctxt.Out.Write(ld.Elfstrdat) if ctxt.LinkMode == ld.LinkExternal { @@ -219,6 +220,7 @@ func asmb2(ctxt *ld.Link) { ld.Asmbelf(ctxt, int64(symo)) } + ctxt.Out.Flush() if *ld.FlagC { fmt.Printf("textsize=%d\n", ld.Segtext.Filelen) fmt.Printf("datsize=%d\n", ld.Segdata.Filelen) diff --git a/src/cmd/link/internal/mips64/asm.go b/src/cmd/link/internal/mips64/asm.go index 0a2a3c11f3..1b2914eea3 100644 --- a/src/cmd/link/internal/mips64/asm.go +++ b/src/cmd/link/internal/mips64/asm.go @@ -223,6 +223,7 @@ func asmb2(ctxt *ld.Link) { default: if ctxt.IsELF { ld.Asmelfsym(ctxt) + ctxt.Out.Flush() ctxt.Out.Write(ld.Elfstrdat) if ctxt.LinkMode == ld.LinkExternal { @@ -232,11 +233,13 @@ func asmb2(ctxt *ld.Link) { case objabi.Hplan9: ld.Asmplan9sym(ctxt) + ctxt.Out.Flush() sym := ctxt.Syms.Lookup("pclntab", 0) if sym != nil { ld.Lcsize = int32(len(sym.P)) ctxt.Out.Write(sym.P) + ctxt.Out.Flush() } } } @@ -265,6 +268,7 @@ func asmb2(ctxt *ld.Link) { ld.Asmbelf(ctxt, int64(symo)) } + ctxt.Out.Flush() if *ld.FlagC { fmt.Printf("textsize=%d\n", ld.Segtext.Filelen) fmt.Printf("datsize=%d\n", ld.Segdata.Filelen) diff --git a/src/cmd/link/internal/ppc64/asm.go b/src/cmd/link/internal/ppc64/asm.go index 0e3a691432..d86738538d 100644 --- a/src/cmd/link/internal/ppc64/asm.go +++ b/src/cmd/link/internal/ppc64/asm.go @@ -1130,6 +1130,7 @@ func asmb2(ctxt *ld.Link) { default: if ctxt.IsELF { ld.Asmelfsym(ctxt) + ctxt.Out.Flush() ctxt.Out.Write(ld.Elfstrdat) if ctxt.LinkMode == ld.LinkExternal { @@ -1139,15 +1140,18 @@ func asmb2(ctxt *ld.Link) { case objabi.Hplan9: ld.Asmplan9sym(ctxt) + ctxt.Out.Flush() sym := ctxt.Syms.Lookup("pclntab", 0) if sym != nil { ld.Lcsize = int32(len(sym.P)) ctxt.Out.Write(sym.P) + ctxt.Out.Flush() } case objabi.Haix: // symtab must be added once sections have been created in ld.Asmbxcoff + ctxt.Out.Flush() } } @@ -1176,6 +1180,7 @@ func asmb2(ctxt *ld.Link) { ld.Asmbxcoff(ctxt, int64(fileoff)) } + ctxt.Out.Flush() if *ld.FlagC { fmt.Printf("textsize=%d\n", ld.Segtext.Filelen) fmt.Printf("datsize=%d\n", ld.Segdata.Filelen) diff --git a/src/cmd/link/internal/riscv64/asm.go b/src/cmd/link/internal/riscv64/asm.go index 51cc5980c8..db3b602b84 100644 --- a/src/cmd/link/internal/riscv64/asm.go +++ b/src/cmd/link/internal/riscv64/asm.go @@ -142,6 +142,7 @@ func asmb2(ctxt *ld.Link) { ctxt.Out.SeekSet(int64(symo)) ld.Asmelfsym(ctxt) + ctxt.Out.Flush() ctxt.Out.Write(ld.Elfstrdat) if ctxt.LinkMode == ld.LinkExternal { @@ -156,6 +157,7 @@ func asmb2(ctxt *ld.Link) { default: ld.Errorf(nil, "unsupported operating system") } + ctxt.Out.Flush() if *ld.FlagC { fmt.Printf("textsize=%d\n", ld.Segtext.Filelen) diff --git a/src/cmd/link/internal/s390x/asm.go b/src/cmd/link/internal/s390x/asm.go index 9b6be28421..5d55a19072 100644 --- a/src/cmd/link/internal/s390x/asm.go +++ b/src/cmd/link/internal/s390x/asm.go @@ -515,6 +515,7 @@ func asmb2(ctxt *ld.Link) { ctxt.Out.SeekSet(int64(symo)) ld.Asmelfsym(ctxt) + ctxt.Out.Flush() ctxt.Out.Write(ld.Elfstrdat) if ctxt.LinkMode == ld.LinkExternal { @@ -530,6 +531,7 @@ func asmb2(ctxt *ld.Link) { ld.Asmbelf(ctxt, int64(symo)) } + ctxt.Out.Flush() if *ld.FlagC { fmt.Printf("textsize=%d\n", ld.Segtext.Filelen) fmt.Printf("datsize=%d\n", ld.Segdata.Filelen) diff --git a/src/cmd/link/internal/wasm/asm.go b/src/cmd/link/internal/wasm/asm.go index 7f8742d008..550ed5bc3c 100644 --- a/src/cmd/link/internal/wasm/asm.go +++ b/src/cmd/link/internal/wasm/asm.go @@ -187,6 +187,8 @@ func asmb2(ctxt *ld.Link) { if !*ld.FlagS { writeNameSec(ctxt, len(hostImports), fns) } + + ctxt.Out.Flush() } func lookupType(sig *wasmFuncType, types *[]*wasmFuncType) uint32 { diff --git a/src/cmd/link/internal/x86/asm.go b/src/cmd/link/internal/x86/asm.go index 7dcdda0fa8..cbc79c987b 100644 --- a/src/cmd/link/internal/x86/asm.go +++ b/src/cmd/link/internal/x86/asm.go @@ -646,6 +646,7 @@ func asmb2(ctxt *ld.Link) { default: if ctxt.IsELF { ld.Asmelfsym(ctxt) + ctxt.Out.Flush() ctxt.Out.Write(ld.Elfstrdat) if ctxt.LinkMode == ld.LinkExternal { @@ -655,11 +656,13 @@ func asmb2(ctxt *ld.Link) { case objabi.Hplan9: ld.Asmplan9sym(ctxt) + ctxt.Out.Flush() sym := ctxt.Syms.Lookup("pclntab", 0) if sym != nil { ld.Lcsize = int32(len(sym.P)) ctxt.Out.Write(sym.P) + ctxt.Out.Flush() } case objabi.Hwindows: @@ -699,4 +702,6 @@ func asmb2(ctxt *ld.Link) { case objabi.Hwindows: ld.Asmbpe(ctxt) } + + ctxt.Out.Flush() }