Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

storage/engine: benchmark MVCCDeleteRange #3105

Merged
merged 2 commits into from
Nov 12, 2015
Merged

storage/engine: benchmark MVCCDeleteRange #3105

merged 2 commits into from
Nov 12, 2015

Conversation

tamird
Copy link
Contributor

@tamird tamird commented Nov 11, 2015

Motivated by #3103. cc @petermattis @mrtracy

$ go test ./storage/engine -run - -bench MVCCDeleteRange 2>/dev/null
PASS
BenchmarkMVCCDeleteRange1Version8Bytes-4          10     190795633 ns/op       2.75 MB/s
BenchmarkMVCCDeleteRange1Version32Bytes-4         10     128830121 ns/op       4.07 MB/s
BenchmarkMVCCDeleteRange1Version256Bytes-4        50      38724836 ns/op      13.54 MB/s
ok      github.com/cockroachdb/cockroach/storage/engine 13.159s

$ go test ./storage/engine -run - -bench MVCCDeleteRange -memprofilerate=1 -memprofile mem.out 2>/dev/null
...
$ go tool pprof --alloc_objects engine.test mem.out
Entering interactive mode (type "help" for commands)
(pprof) top10
934147 of 1136965 total (82.16%)
Dropped 142 nodes (cum <= 5684)
Showing top 10 nodes out of 34 (cum >= 534539)
      flat  flat%   sum%        cum   cum%
    133418 11.73% 11.73%     133418 11.73%  github.com/cockroachdb/cockroach/roachpb.(*Value).Unmarshal
    133418 11.73% 23.47%     133418 11.73%  github.com/cockroachdb/cockroach/storage/engine.(*MVCCMetadata).Marshal
    133418 11.73% 35.20%     133418 11.73%  github.com/cockroachdb/cockroach/storage/engine.(*MVCCValue).Marshal
    133418 11.73% 46.94%     333545 29.34%  github.com/cockroachdb/cockroach/storage/engine.(*RocksDB).getProtoInternal
     66913  5.89% 52.82%     401833 35.34%  github.com/cockroachdb/cockroach/storage/engine.setupMVCCData
     66726  5.87% 58.69%      66726  5.87%  github.com/cockroachdb/cockroach/storage/engine.mvccEncodeTimestamp
     66709  5.87% 64.56%      66744  5.87%  github.com/cockroachdb/cockroach/roachpb.(*Value).InitChecksum
     66709  5.87% 70.43%     200127 17.60%  github.com/cockroachdb/cockroach/storage/engine.(*MVCCValue).Unmarshal
     66709  5.87% 76.29%      66709  5.87%  github.com/cockroachdb/cockroach/storage/engine.(*rocksDBBatch).GetProto
     66709  5.87% 82.16%     534539 47.01%  github.com/cockroachdb/cockroach/storage/engine.MVCCIterate

Review on Reviewable

b.StopTimer()

for i := 0; i < b.N; i++ {
rocksdb, stopper := setupMVCCData(1, numKeys, valueBytes, false, b)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Continually recreating the data is likely to be slow. I think you get rid of the changes to setupMVCCData and instead copy the directory and its contents on each loop iteration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@petermattis
Copy link
Collaborator

MVCCDeleteRange is currently implemented as MVCCScan followed by MVCCDelete. I imagine there is a big win from integrating the scan with the delete.

@petermattis
Copy link
Collaborator

LGTM, though do you mind waiting to merge until #3096 goes in (just needs to pass tests)? I'd also be interested to know if #3096 affects this benchmark (it should). Btw, when you compare old and new, run each benchmark 10 times (go test -count 10) and use benchstat to compare the output.

@tamird
Copy link
Contributor Author

tamird commented Nov 11, 2015

Sure, I can wait. Not sure what you mean about old and new, since there is only one result set here.

@petermattis
Copy link
Collaborator

Old = before #3096 is merged.
New = after #3096 is merged.

stopper.Stop()
}

locDirty := loc + "_dirty"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking you would only make the copy in BenchmarkMVCCDeleteRange. Performing the copy for every benchmark seems excessive.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@tamird
Copy link
Contributor Author

tamird commented Nov 12, 2015

Results of rebasing on #3096:

name                               old time/op    new time/op    delta
MVCCDeleteRange1Version8Bytes-4       138ms ± 0%     132ms ± 1%  -4.79%   (p=0.000 n=8+10)
MVCCDeleteRange1Version32Bytes-4     98.1ms ± 1%    93.4ms ± 1%  -4.79%  (p=0.000 n=10+10)
MVCCDeleteRange1Version256Bytes-4    27.2ms ± 1%    25.9ms ± 1%  -4.77%   (p=0.000 n=10+9)

name                               old speed      new speed      delta
MVCCDeleteRange1Version8Bytes-4    3.80MB/s ± 1%  3.98MB/s ± 1%  +4.75%   (p=0.000 n=10+8)
MVCCDeleteRange1Version32Bytes-4   5.34MB/s ± 1%  5.61MB/s ± 1%  +5.01%  (p=0.000 n=10+10)
MVCCDeleteRange1Version256Bytes-4  19.3MB/s ± 1%  20.2MB/s ± 1%  +5.00%   (p=0.000 n=10+9)

@tbg
Copy link
Member

tbg commented Nov 12, 2015

LGTM


Reviewed 1 of 1 files at r1, 2 of 2 files at r2, 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 3 unresolved discussions.


storage/engine/rocksdb_test.go, line 511 [r3] (raw file):
does setting those attributes make any difference?


Comments from the review on Reviewable.io

```
$ go test ./storage/engine -run - -bench MVCCDeleteRange 2>/dev/null
PASS
BenchmarkMVCCDeleteRange1Version8Bytes-4          10     182424913 ns/op       2.87 MB/s
BenchmarkMVCCDeleteRange1Version32Bytes-4         10     111075871 ns/op       4.72 MB/s
BenchmarkMVCCDeleteRange1Version256Bytes-4        50      32043937 ns/op      16.36 MB/s
ok      github.com/cockroachdb/cockroach/storage/engine 5.719s

$ go test ./storage/engine -run - -bench MVCCDeleteRange -memprofilerate=1 -memprofile mem.out 2>/dev/null
...
$ go tool pprof --alloc_objects engine.test mem.out
Entering interactive mode (type "help" for commands)
(pprof) top10
934147 of 1136965 total (82.16%)
Dropped 142 nodes (cum <= 5684)
Showing top 10 nodes out of 34 (cum >= 534539)
      flat  flat%   sum%        cum   cum%
    133418 11.73% 11.73%     133418 11.73%  github.com/cockroachdb/cockroach/roachpb.(*Value).Unmarshal
    133418 11.73% 23.47%     133418 11.73%  github.com/cockroachdb/cockroach/storage/engine.(*MVCCMetadata).Marshal
    133418 11.73% 35.20%     133418 11.73%  github.com/cockroachdb/cockroach/storage/engine.(*MVCCValue).Marshal
    133418 11.73% 46.94%     333545 29.34%  github.com/cockroachdb/cockroach/storage/engine.(*RocksDB).getProtoInternal
     66913  5.89% 52.82%     401833 35.34%  github.com/cockroachdb/cockroach/storage/engine.setupMVCCData
     66726  5.87% 58.69%      66726  5.87%  github.com/cockroachdb/cockroach/storage/engine.mvccEncodeTimestamp
     66709  5.87% 64.56%      66744  5.87%  github.com/cockroachdb/cockroach/roachpb.(*Value).InitChecksum
     66709  5.87% 70.43%     200127 17.60%  github.com/cockroachdb/cockroach/storage/engine.(*MVCCValue).Unmarshal
     66709  5.87% 76.29%      66709  5.87%  github.com/cockroachdb/cockroach/storage/engine.(*rocksDBBatch).GetProto
     66709  5.87% 82.16%     534539 47.01%  github.com/cockroachdb/cockroach/storage/engine.MVCCIterate
```
@tamird
Copy link
Contributor Author

tamird commented Nov 12, 2015

Review status: all files reviewed at latest revision, 3 unresolved discussions.


storage/engine/rocksdb_test.go, line 511 [r3] (raw file):
No, they do nothing. Looks like a historical accident that these attributes are on the engine rather than the store. Removed.


Comments from the review on Reviewable.io

tamird added a commit that referenced this pull request Nov 12, 2015
storage/engine: benchmark MVCCDeleteRange
@tamird tamird merged commit 6af5e79 into cockroachdb:master Nov 12, 2015
@tamird tamird deleted the mvccdelete-bench branch November 12, 2015 17:45
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants