Skip to content

Commit

Permalink
Revert "Introduce zed.Arena (#5118)" and "sort: Fix spill garbage col…
Browse files Browse the repository at this point in the history
…lection bug (#5239)" (#5278)

This reverts commits ce9626e and
1ffe14f.

A zed.Value allocated by a zed.Arena holds an untyped pointer to the
arena in zed.Value.a, hiding the arena from the garbage collector and
requiring maintenance elsewhere of a typed pointer that lives as long as
the zed.Value.  Failure to do so reliably has so far led to three
distinct panics not caught by automated tests, and more are likely to
appear.

At the same time, arenas haven't produced the anticipated performance
benefit, in part because they introduce new bottlenecks without obvious
fixes.

In this light and given current priorities, time spent fixing arena bugs
isn't well spent, so this change removes zed.Arena.
  • Loading branch information
nwt authored Sep 16, 2024
1 parent 43053bf commit 427a57b
Show file tree
Hide file tree
Showing 210 changed files with 1,804 additions and 2,497 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ endif
#PEG_DEP=peg

vet:
@go vet -unsafeptr=false ./...
@go vet ./...

fmt:
gofmt -s -w .
Expand Down
5 changes: 1 addition & 4 deletions api/client/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"net/http/httptrace"
"time"

"github.com/brimdata/zed"
"github.com/brimdata/zed/api"
"github.com/brimdata/zed/zio"
"github.com/brimdata/zed/zio/zngio"
Expand Down Expand Up @@ -96,9 +95,7 @@ func (r *Request) reader() (io.Reader, error) {
}
m := zson.NewZNGMarshaler()
m.Decorate(zson.StylePackage)
arena := zed.NewArena()
defer arena.Unref()
val, err := m.Marshal(arena, r.Body)
val, err := m.Marshal(r.Body)
if err != nil {
return nil, err
}
Expand Down
4 changes: 1 addition & 3 deletions api/queryio/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,7 @@ func marshalControl(zctrl *zbuf.Control) (any, error) {
if ctrl.Format != zngio.ControlFormatZSON {
return nil, fmt.Errorf("unsupported app encoding: %v", ctrl.Format)
}
arena := zed.NewArena()
defer arena.Unref()
value, err := zson.ParseValue(zed.NewContext(), arena, string(ctrl.Bytes))
value, err := zson.ParseValue(zed.NewContext(), string(ctrl.Bytes))
if err != nil {
return nil, fmt.Errorf("unable to parse Zed control message: %w (%s)", err, ctrl.Bytes)
}
Expand Down
4 changes: 1 addition & 3 deletions api/queryio/zjson_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ func TestZJSONWriter(t *testing.T) {
w := queryio.NewZJSONWriter(&buf)
err := w.WriteControl(api.QueryChannelSet{Channel: "main"})
require.NoError(t, err)
arena := zed.NewArena()
defer arena.Unref()
err = w.Write(zson.MustParseValue(zed.NewContext(), arena, record))
err = w.Write(zson.MustParseValue(zed.NewContext(), record))
require.NoError(t, err)
err = w.WriteControl(api.QueryChannelEnd{Channel: "main"})
require.NoError(t, err)
Expand Down
6 changes: 1 addition & 5 deletions api/queryio/zng.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"io"

"github.com/brimdata/zed"
"github.com/brimdata/zed/zio"
"github.com/brimdata/zed/zio/zngio"
"github.com/brimdata/zed/zio/zsonio"
Expand All @@ -14,7 +13,6 @@ import (
type ZNGWriter struct {
*zngio.Writer
marshaler *zson.MarshalZNGContext
arena *zed.Arena
}

var _ controlWriter = (*ZJSONWriter)(nil)
Expand All @@ -25,13 +23,11 @@ func NewZNGWriter(w io.Writer) *ZNGWriter {
return &ZNGWriter{
Writer: zngio.NewWriter(zio.NopCloser(w)),
marshaler: m,
arena: zed.NewArena(),
}
}

func (w *ZNGWriter) WriteControl(v interface{}) error {
w.arena.Reset()
val, err := w.marshaler.Marshal(w.arena, v)
val, err := w.marshaler.Marshal(v)
if err != nil {
return err
}
Expand Down
219 changes: 0 additions & 219 deletions arena.go

This file was deleted.

2 changes: 1 addition & 1 deletion cli/zq/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func (c *Command) Run(args []string) error {
local := storage.NewLocalEngine()
var readers []zio.Reader
if null {
readers = []zio.Reader{zbuf.NewArray(nil, []zed.Value{zed.Null})}
readers = []zio.Reader{zbuf.NewArray([]zed.Value{zed.Null})}
} else {
readers, err = c.inputFlags.Open(ctx, zctx, local, paths, c.stopErr)
if err != nil {
Expand Down
5 changes: 1 addition & 4 deletions cmd/zed/dev/dig/frames/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ func (c *Command) Run(args []string) error {
type metaReader struct {
reader *reader
marshaler *zson.MarshalZNGContext
arena *zed.Arena
}

var _ zio.Reader = (*metaReader)(nil)
Expand All @@ -86,7 +85,6 @@ func newMetaReader(r io.Reader) *metaReader {
return &metaReader{
reader: &reader{reader: bufio.NewReader(r)},
marshaler: zson.NewZNGMarshaler(),
arena: zed.NewArena(),
}
}

Expand Down Expand Up @@ -118,8 +116,7 @@ func (m *metaReader) Read() (*zed.Value, error) {
if f == nil || err != nil {
return nil, err
}
m.arena.Reset()
val, err := m.marshaler.Marshal(m.arena, f)
val, err := m.marshaler.Marshal(f)
return &val, err
}

Expand Down
5 changes: 2 additions & 3 deletions cmd/zed/dev/vector/agg/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,15 @@ func (c *Command) Run(args []string) error {
return err
}
field := args[1]
zctx := zed.NewContext()
local := storage.NewLocalEngine()
cache := vcache.NewCache(local)
object, err := cache.Fetch(ctx, zctx, uri, ksuid.Nil)
object, err := cache.Fetch(ctx, uri, ksuid.Nil)
if err != nil {
return err
}
defer object.Close()
//XXX nil puller
agg := op.NewCountByString(zctx, nil, field)
agg := op.NewCountByString(zed.NewContext(), nil, field)
writer, err := c.outputFlags.Open(ctx, local)
if err != nil {
return err
Expand Down
5 changes: 2 additions & 3 deletions cmd/zed/dev/vector/copy/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,8 @@ func (c *Command) Run(args []string) error {
if err != nil {
return err
}
zctx := zed.NewContext()
local := storage.NewLocalEngine()
object, err := vcache.NewObject(ctx, zctx, local, uri)
object, err := vcache.NewObject(ctx, local, uri)
if err != nil {
return err
}
Expand All @@ -68,7 +67,7 @@ func (c *Command) Run(args []string) error {
if err != nil {
return err
}
puller := vam.NewProjection(zctx, object, nil)
puller := vam.NewProjection(zed.NewContext(), object, nil)
if err := zbuf.CopyPuller(writer, puller); err != nil {
writer.Close()
return err
Expand Down
5 changes: 2 additions & 3 deletions cmd/zed/dev/vector/project/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,14 @@ func (c *Command) Run(args []string) error {
for _, dotted := range args[1:] {
paths = append(paths, field.Dotted(dotted))
}
zctx := zed.NewContext()
local := storage.NewLocalEngine()
cache := vcache.NewCache(local)
object, err := cache.Fetch(ctx, zctx, uri, ksuid.Nil)
object, err := cache.Fetch(ctx, uri, ksuid.Nil)
if err != nil {
return err
}
defer object.Close()
projection := vam.NewProjection(zctx, object, paths)
projection := vam.NewProjection(zed.NewContext(), object, paths)
writer, err := c.outputFlags.Open(ctx, local)
if err != nil {
return err
Expand Down
Loading

0 comments on commit 427a57b

Please sign in to comment.