Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
SunRunAway committed Sep 11, 2019
1 parent 7d42f05 commit 9373cdd
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 43 deletions.
37 changes: 21 additions & 16 deletions util/chunk/disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (l *ListInDisk) GetRow(ptr RowPtr) (row Row, err error) {
format := rowInDisk{numCol: len(l.fieldTypes)}
_, err = format.ReadFrom(bufReader)
if err != nil {
return
return row, err
}
row = format.toMutRow(l.fieldTypes).ToRow()
return row, err
Expand Down Expand Up @@ -179,7 +179,7 @@ func (chk *chunkInDisk) WriteTo(w io.Writer) (written int64, err error) {
format = convertFromRow(chk.GetRow(rowIdx), format)
chk.offsetsOfRows = append(chk.offsetsOfRows, chk.offWrite+written)

n, err = chk.writeRowTo(w, format)
n, err = rowInDisk{diskFormatRow: *format}.WriteTo(w)
written += n
if err != nil {
return
Expand All @@ -188,15 +188,24 @@ func (chk *chunkInDisk) WriteTo(w io.Writer) (written int64, err error) {
return
}

// writeRowTo serializes a row of the chunk into the format of
// getOffsetsOfRows gets the offset of each row.
func (chk *chunkInDisk) getOffsetsOfRows() []int64 { return chk.offsetsOfRows }

// rowInDisk represents a Row in format of diskFormatRow.
type rowInDisk struct {
numCol int
diskFormatRow
}

// WriteTo serializes a row of the chunk into the format of
// diskFormatRow, and writes to w.
func (chk *chunkInDisk) writeRowTo(w io.Writer, format *diskFormatRow) (written int64, err error) {
n, err := w.Write(i64SliceToBytes(format.sizesOfColumns))
func (row rowInDisk) WriteTo(w io.Writer) (written int64, err error) {
n, err := w.Write(i64SliceToBytes(row.sizesOfColumns))
written += int64(n)
if err != nil {
return
}
for _, data := range format.cells {
for _, data := range row.cells {
n, err = w.Write(data)
written += int64(n)
if err != nil {
Expand All @@ -206,14 +215,6 @@ func (chk *chunkInDisk) writeRowTo(w io.Writer, format *diskFormatRow) (written
return
}

func (chk *chunkInDisk) getOffsetsOfRows() []int64 { return chk.offsetsOfRows }

// rowInDisk represents a Row in format of diskFormatRow.
type rowInDisk struct {
numCol int
diskFormatRow
}

// ReadFrom reads data of r, deserializes it from the format of diskFormatRow
// into Row.
func (row *rowInDisk) ReadFrom(r io.Reader) (n int64, err error) {
Expand Down Expand Up @@ -247,10 +248,14 @@ type diskFormatRow struct {
// sizesOfColumns stores the size of each column in a row.
// -1 means the value of this column is null.
sizesOfColumns []int64 // -1 means null
cells [][]byte
// cells represents raw data of not-null columns in one row.
// In convertFromRow, data from Row is shallow copied to cells.
// In toMutRow, data in cells is shallow copied to MutRow.
cells [][]byte
}

// convertFromRow serializes one row of chunk to diskFormatRow
// convertFromRow serializes one row of chunk to diskFormatRow, then
// we can use diskFormatRow to write to disk.
func convertFromRow(row Row, reuse *diskFormatRow) (format *diskFormatRow) {
numCols := row.Chunk().NumCols()
if reuse != nil {
Expand Down
31 changes: 13 additions & 18 deletions util/chunk/disk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@ import (
"os"
"testing"

"github.com/cznic/mathutil"
"github.com/pingcap/check"
"github.com/pingcap/parser/mysql"
"github.com/pingcap/tidb/types"
"github.com/pingcap/tidb/types/json"
)

func initChunks(numChk, numRow int) ([]*Chunk, []*types.FieldType, error) {
func initChunks(numChk, numRow int) ([]*Chunk, []*types.FieldType) {
fields := []*types.FieldType{
types.NewFieldType(mysql.TypeVarString),
types.NewFieldType(mysql.TypeLonglong),
Expand All @@ -36,7 +37,7 @@ func initChunks(numChk, numRow int) ([]*Chunk, []*types.FieldType, error) {

chks := make([]*Chunk, 0, numChk)
for chkIdx := 0; chkIdx < numChk; chkIdx++ {
chk := NewChunkWithCapacity(fields, 2)
chk := NewChunkWithCapacity(fields, numRow)
for rowIdx := 0; rowIdx < numRow; rowIdx++ {
data := int64(chkIdx*numRow + rowIdx)
chk.AppendString(0, fmt.Sprint(data))
Expand All @@ -51,20 +52,17 @@ func initChunks(numChk, numRow int) ([]*Chunk, []*types.FieldType, error) {
}
chks = append(chks, chk)
}
return chks, fields, nil
return chks, fields
}

func (s *testChunkSuite) TestListInDisk(c *check.C) {
numChk, numRow := 2, 2
chks, fields, err := initChunks(numChk, numRow)
if err != nil {
c.Fatal(err)
}
chks, fields := initChunks(numChk, numRow)
l := NewListInDisk(fields)
defer func() {
err := l.Close()
c.Check(err, check.IsNil)
c.Check(l.disk, check.Not(check.IsNil))
c.Check(l.disk, check.NotNil)
_, err = os.Stat(l.disk.Name())
c.Check(os.IsNotExist(err), check.IsTrue)
}()
Expand All @@ -87,10 +85,7 @@ func (s *testChunkSuite) TestListInDisk(c *check.C) {

func BenchmarkListInDiskAdd(b *testing.B) {
numChk, numRow := 1, 2
chks, fields, err := initChunks(numChk, numRow)
if err != nil {
b.Fatal(err)
}
chks, fields := initChunks(numChk, numRow)
chk := chks[0]
l := NewListInDisk(fields)
defer l.Close()
Expand All @@ -106,10 +101,7 @@ func BenchmarkListInDiskAdd(b *testing.B) {

func BenchmarkListInDiskGetRow(b *testing.B) {
numChk, numRow := 10000, 2
chks, fields, err := initChunks(numChk, numRow)
if err != nil {
b.Fatal(err)
}
chks, fields := initChunks(numChk, numRow)
l := NewListInDisk(fields)
defer l.Close()
for _, chk := range chks {
Expand All @@ -120,15 +112,18 @@ func BenchmarkListInDiskGetRow(b *testing.B) {
}
rand.Seed(0)
ptrs := make([]RowPtr, 0, b.N)
for i := 0; i < b.N; i++ {
for i := 0; i < mathutil.Min(b.N, 10000); i++ {
ptrs = append(ptrs, RowPtr{
ChkIdx: rand.Uint32() % uint32(numChk),
RowIdx: rand.Uint32() % uint32(numRow),
})
}
for i := 10000; i < cap(ptrs); i++ {
ptrs = append(ptrs, ptrs[i%10000])
}
b.ResetTimer()
for i := 0; i < b.N; i++ {
_, err = l.GetRow(ptrs[i])
_, err := l.GetRow(ptrs[i])
if err != nil {
b.Fatal(err)
}
Expand Down
16 changes: 7 additions & 9 deletions util/chunk/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"testing"
"time"

"github.com/cznic/mathutil"
"github.com/pingcap/check"
"github.com/pingcap/parser/mysql"
"github.com/pingcap/tidb/types"
Expand Down Expand Up @@ -222,10 +223,7 @@ func BenchmarkPreAllocChunk(b *testing.B) {

func BenchmarkListAdd(b *testing.B) {
numChk, numRow := 1, 2
chks, fields, err := initChunks(numChk, numRow)
if err != nil {
b.Fatal(err)
}
chks, fields := initChunks(numChk, numRow)
chk := chks[0]
l := NewList(fields, numRow, numRow)

Expand All @@ -237,22 +235,22 @@ func BenchmarkListAdd(b *testing.B) {

func BenchmarkListGetRow(b *testing.B) {
numChk, numRow := 10000, 2
chks, fields, err := initChunks(numChk, numRow)
if err != nil {
b.Fatal(err)
}
chks, fields := initChunks(numChk, numRow)
l := NewList(fields, numRow, numRow)
for _, chk := range chks {
l.Add(chk)
}
rand.Seed(0)
ptrs := make([]RowPtr, 0, b.N)
for i := 0; i < b.N; i++ {
for i := 0; i < mathutil.Min(b.N, 10000); i++ {
ptrs = append(ptrs, RowPtr{
ChkIdx: rand.Uint32() % uint32(numChk),
RowIdx: rand.Uint32() % uint32(numRow),
})
}
for i := 10000; i < cap(ptrs); i++ {
ptrs = append(ptrs, ptrs[i%10000])
}
b.ResetTimer()
for i := 0; i < b.N; i++ {
l.GetRow(ptrs[i])
Expand Down

0 comments on commit 9373cdd

Please sign in to comment.