Skip to content

Commit

Permalink
Revert "Add comments and test for 64-bit field alignment (#418)"
Browse files Browse the repository at this point in the history
This reverts commit 91091b4.
  • Loading branch information
lizthegrey authored Jan 6, 2020
1 parent 91091b4 commit 5b69cdc
Show file tree
Hide file tree
Showing 21 changed files with 20 additions and 330 deletions.
6 changes: 2 additions & 4 deletions api/core/number_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,12 @@ func TestNumber(t *testing.T) {
cmpsForPos := [3]int{1, 1, 0}

type testcase struct {
// n needs to be aligned for 64-bit atomic operations.
n Number
// nums needs to be aligned for 64-bit atomic operations.
nums [3]Number
n Number
kind NumberKind
pos bool
zero bool
neg bool
nums [3]Number
cmps [3]int
}
testcases := []testcase{
Expand Down
24 changes: 0 additions & 24 deletions api/metric/alignment_test.go

This file was deleted.

3 changes: 1 addition & 2 deletions api/metric/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,8 @@ type MeasureOptionApplier interface {
// values. Instances of this type should be created by instruments
// (e.g., Int64Counter.Measurement()).
type Measurement struct {
// number needs to be aligned for 64-bit atomic operations.
number core.Number
instrument InstrumentImpl
number core.Number
}

// Instrument returns the instrument that created this measurement.
Expand Down
6 changes: 2 additions & 4 deletions internal/metric/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,9 @@ type (
}

Batch struct {
// Measurement needs to be aligned for 64-bit atomic operations.
Measurements []Measurement
Ctx context.Context
LabelSet *LabelSet
Measurements []Measurement
}

MeterProvider struct {
Expand All @@ -59,9 +58,8 @@ type (
Kind int8

Measurement struct {
// Number needs to be aligned for 64-bit atomic operations.
Number core.Number
Instrument *Instrument
Number core.Number
}
)

Expand Down
28 changes: 0 additions & 28 deletions internal/metric/mock_test.go

This file was deleted.

57 changes: 0 additions & 57 deletions internal/testing/alignment.go

This file was deleted.

8 changes: 3 additions & 5 deletions internal/trace/mock_tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,12 @@ import (
// It only supports ChildOf option. SpanId is atomically increased every time a
// new span is created.
type MockTracer struct {
// Sampled specifies if the new span should be sampled or not.
Sampled bool

// StartSpanID is used to initialize spanId. It is incremented by one
// every time a new span is created.
//
// StartSpanID has to be aligned for 64-bit atomic operations.
StartSpanID *uint64

// Sampled specifies if the new span should be sampled or not.
Sampled bool
}

var _ apitrace.Tracer = (*MockTracer)(nil)
Expand Down
24 changes: 0 additions & 24 deletions internal/trace/mock_tracer_test.go

This file was deleted.

3 changes: 1 addition & 2 deletions sdk/metric/aggregator/array/array.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,10 @@ import (

type (
Aggregator struct {
// ckptSum needs to be aligned for 64-bit atomic operations.
ckptSum core.Number
lock sync.Mutex
current points
checkpoint points
ckptSum core.Number
}

points []core.Number
Expand Down
18 changes: 0 additions & 18 deletions sdk/metric/aggregator/array/array_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,34 +18,16 @@ import (
"context"
"fmt"
"math"
"os"
"testing"
"unsafe"

"github.com/stretchr/testify/require"

"go.opentelemetry.io/otel/api/core"
ottest "go.opentelemetry.io/otel/internal/testing"
export "go.opentelemetry.io/otel/sdk/export/metric"
"go.opentelemetry.io/otel/sdk/export/metric/aggregator"
"go.opentelemetry.io/otel/sdk/metric/aggregator/test"
)

// Ensure struct alignment prior to running tests.
func TestMain(m *testing.M) {
fields := []ottest.FieldOffset{
{
Name: "Aggregator.ckptSum",
Offset: unsafe.Offsetof(Aggregator{}.ckptSum),
},
}
if !ottest.Aligned8Byte(fields, os.Stderr) {
os.Exit(1)
}

os.Exit(m.Run())
}

type updateTest struct {
count int
absolute bool
Expand Down
2 changes: 0 additions & 2 deletions sdk/metric/aggregator/counter/counter.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,9 @@ import (
// Aggregator aggregates counter events.
type Aggregator struct {
// current holds current increments to this counter record
// current needs to be aligned for 64-bit atomic operations.
current core.Number

// checkpoint is a temporary used during Checkpoint()
// checkpoint needs to be aligned for 64-bit atomic operations.
checkpoint core.Number
}

Expand Down
22 changes: 0 additions & 22 deletions sdk/metric/aggregator/counter/counter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,39 +16,17 @@ package counter

import (
"context"
"os"
"testing"
"unsafe"

"github.com/stretchr/testify/require"

"go.opentelemetry.io/otel/api/core"
ottest "go.opentelemetry.io/otel/internal/testing"
export "go.opentelemetry.io/otel/sdk/export/metric"
"go.opentelemetry.io/otel/sdk/metric/aggregator/test"
)

const count = 100

// Ensure struct alignment prior to running tests.
func TestMain(m *testing.M) {
fields := []ottest.FieldOffset{
{
Name: "Aggregator.current",
Offset: unsafe.Offsetof(Aggregator{}.current),
},
{
Name: "Aggregator.checkpoint",
Offset: unsafe.Offsetof(Aggregator{}.checkpoint),
},
}
if !ottest.Aligned8Byte(fields, os.Stderr) {
os.Exit(1)
}

os.Exit(m.Run())
}

func TestCounterMonotonic(t *testing.T) {
ctx := context.Background()

Expand Down
2 changes: 0 additions & 2 deletions sdk/metric/aggregator/gauge/gauge.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ type (
// a sequence number to determine the winner of a race.
gaugeData struct {
// value is the int64- or float64-encoded Set() data
//
// value needs to be aligned for 64-bit atomic operations.
value core.Number

// timestamp indicates when this record was submitted.
Expand Down
18 changes: 0 additions & 18 deletions sdk/metric/aggregator/gauge/gauge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,11 @@ package gauge
import (
"context"
"math/rand"
"os"
"testing"
"unsafe"

"github.com/stretchr/testify/require"

"go.opentelemetry.io/otel/api/core"
ottest "go.opentelemetry.io/otel/internal/testing"
export "go.opentelemetry.io/otel/sdk/export/metric"
"go.opentelemetry.io/otel/sdk/export/metric/aggregator"
"go.opentelemetry.io/otel/sdk/metric/aggregator/test"
Expand All @@ -34,21 +31,6 @@ const count = 100

var _ export.Aggregator = &Aggregator{}

// Ensure struct alignment prior to running tests.
func TestMain(m *testing.M) {
fields := []ottest.FieldOffset{
{
Name: "gaugeData.value",
Offset: unsafe.Offsetof(gaugeData{}.value),
},
}
if !ottest.Aligned8Byte(fields, os.Stderr) {
os.Exit(1)
}

os.Exit(m.Run())
}

func TestGaugeNonMonotonic(t *testing.T) {
ctx := context.Background()

Expand Down
5 changes: 1 addition & 4 deletions sdk/metric/aggregator/minmaxsumcount/mmsc.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,12 @@ type (
// Aggregator aggregates measure events, keeping only the max,
// sum, and count.
Aggregator struct {
// current has to be aligned for 64-bit atomic operations.
current state
// checkpoint has to be aligned for 64-bit atomic operations.
current state
checkpoint state
kind core.NumberKind
}

state struct {
// all fields have to be aligned for 64-bit atomic operations.
count core.Number
sum core.Number
min core.Number
Expand Down
Loading

0 comments on commit 5b69cdc

Please sign in to comment.