Skip to content

Commit

Permalink
Add buffer pool and stateful errors (#42)
Browse files Browse the repository at this point in the history
* Make Reader and Close on Writers errors stateful, so they can't be silently ignored.
* Add buffer pool, to help small operations.
* Make benchmarks parallel, so they can be controlled with `-cpu` param.

Stateful errors are required for buffers and are good practice anyway.

```
benchmark                             old ns/op     new ns/op     delta
BenchmarkEncryptReader_8KB            7040          1754          -75.09%
BenchmarkEncryptReader_8KB-32         5711          564           -90.13%
BenchmarkEncryptReader_64KB           23739         17406         -26.68%
BenchmarkEncryptReader_64KB-32        8358          1680          -79.90%
BenchmarkEncryptReader_512KB          143045        135148        -5.52%
BenchmarkEncryptReader_512KB-32       17855         9144          -48.79%
BenchmarkEncryptReader_1MB            274863        266411        -3.07%
BenchmarkEncryptReader_1MB-32         28966         17709         -38.86%
BenchmarkDecryptReader_8KB            8390          2036          -75.73%
BenchmarkDecryptReader_8KB-32         5852          700           -88.03%
BenchmarkDecryptReader_64KB           24062         17358         -27.86%
BenchmarkDecryptReader_64KB-32        6742          2038          -69.77%
BenchmarkDecryptReader_512KB          136199        128197        -5.88%
BenchmarkDecryptReader_512KB-32       17214         9722          -43.52%
BenchmarkDecryptReader_1MB            261589        256942        -1.78%
BenchmarkDecryptReader_1MB-32         29842         18367         -38.45%
BenchmarkDecryptReaderAt_8KB          16163         3108          -80.77%
BenchmarkDecryptReaderAt_8KB-32       11139         1011          -90.92%
BenchmarkDecryptReaderAt_64KB         48030         33390         -30.48%
BenchmarkDecryptReaderAt_64KB-32      12682         2859          -77.46%
BenchmarkDecryptReaderAt_512KB        135685        122129        -9.99%
BenchmarkDecryptReaderAt_512KB-32     21756         8825          -59.44%
BenchmarkDecryptReaderAt_1MB          253436        241940        -4.54%
BenchmarkDecryptReaderAt_1MB-32       29090         17011         -41.52%
BenchmarkEncryptWriter_8KB            8110          1721          -78.78%
BenchmarkEncryptWriter_8KB-32         5590          460           -91.77%
BenchmarkEncryptWriter_64KB           23016         17731         -22.96%
BenchmarkEncryptWriter_64KB-32        6519          1212          -81.41%
BenchmarkEncryptWriter_512KB          128791        125068        -2.89%
BenchmarkEncryptWriter_512KB-32       9761          8861          -9.22%
BenchmarkEncryptWriter_1MB            248845        244758        -1.64%
BenchmarkEncryptWriter_1MB-32         18768         17645         -5.98%
BenchmarkDecryptWriter_8KB            10045         1721          -82.87%
BenchmarkDecryptWriter_8KB-32         10071         612           -93.93%
BenchmarkDecryptWriter_64KB           25237         16298         -35.42%
BenchmarkDecryptWriter_64KB-32        7284          1506          -79.32%
BenchmarkDecryptWriter_512KB          130734        122230        -6.50%
BenchmarkDecryptWriter_512KB-32       14006         8581          -38.73%
BenchmarkDecryptWriter_1MB            249146        240821        -3.34%
BenchmarkDecryptWriter_1MB-32         22672         17055         -24.78%

benchmark                             old MB/s     new MB/s     speedup
BenchmarkEncryptReader_8KB            145.45       583.65       4.01x
BenchmarkEncryptReader_8KB-32         179.29       1816.31      10.13x
BenchmarkEncryptReader_64KB           2760.74      3765.09      1.36x
BenchmarkEncryptReader_64KB-32        7841.21      38999.69     4.97x
BenchmarkEncryptReader_512KB          3665.21      3879.36      1.06x
BenchmarkEncryptReader_512KB-32       29363.77     57335.81     1.95x
BenchmarkEncryptReader_1MB            3814.91      3935.93      1.03x
BenchmarkEncryptReader_1MB-32         36200.60     59211.25     1.64x
BenchmarkDecryptReader_8KB            122.05       503.07       4.12x
BenchmarkDecryptReader_8KB-32         174.98       1461.79      8.35x
BenchmarkDecryptReader_64KB           2723.60      3775.56      1.39x
BenchmarkDecryptReader_64KB-32        9720.77      32154.53     3.31x
BenchmarkDecryptReader_512KB          3849.43      4089.72      1.06x
BenchmarkDecryptReader_512KB-32       30457.05     53925.25     1.77x
BenchmarkDecryptReader_1MB            4008.49      4080.99      1.02x
BenchmarkDecryptReader_1MB-32         35137.48     57088.75     1.62x
BenchmarkDecryptReaderAt_8KB          63.36        329.47       5.20x
BenchmarkDecryptReaderAt_8KB-32       91.93        1012.87      11.02x
BenchmarkDecryptReaderAt_64KB         1364.47      1962.73      1.44x
BenchmarkDecryptReaderAt_64KB-32      5167.56      22921.02     4.44x
BenchmarkDecryptReaderAt_512KB        3864.01      4292.92      1.11x
BenchmarkDecryptReaderAt_512KB-32     24098.26     59407.79     2.47x
BenchmarkDecryptReaderAt_1MB          4137.44      4334.04      1.05x
BenchmarkDecryptReaderAt_1MB-32       36046.33     61642.61     1.71x
BenchmarkEncryptWriter_8KB            126.27       594.86       4.71x
BenchmarkEncryptWriter_8KB-32         183.17       2226.33      12.15x
BenchmarkEncryptWriter_64KB           2847.45      3696.05      1.30x
BenchmarkEncryptWriter_64KB-32        10052.84     54070.81     5.38x
BenchmarkEncryptWriter_512KB          4070.85      4192.01      1.03x
BenchmarkEncryptWriter_512KB-32       53709.97     59168.98     1.10x
BenchmarkEncryptWriter_1MB            4213.78      4284.14      1.02x
BenchmarkEncryptWriter_1MB-32         55871.68     59426.78     1.06x
BenchmarkDecryptWriter_8KB            101.94       594.91       5.84x
BenchmarkDecryptWriter_8KB-32         101.68       1673.98      16.46x
BenchmarkDecryptWriter_64KB           2596.84      4021.05      1.55x
BenchmarkDecryptWriter_64KB-32        8996.95      43507.52     4.84x
BenchmarkDecryptWriter_512KB          4010.34      4289.34      1.07x
BenchmarkDecryptWriter_512KB-32       37431.90     61100.09     1.63x
BenchmarkDecryptWriter_1MB            4208.68      4354.17      1.03x
BenchmarkDecryptWriter_1MB-32         46249.03     61480.32     1.33x

benchmark                             old allocs     new allocs     delta
BenchmarkEncryptReader_8KB            12             11             -8.33%
BenchmarkEncryptReader_8KB-32         12             11             -8.33%
BenchmarkEncryptReader_64KB           12             11             -8.33%
BenchmarkEncryptReader_64KB-32        12             11             -8.33%
BenchmarkEncryptReader_512KB          19             18             -5.26%
BenchmarkEncryptReader_512KB-32       19             18             -5.26%
BenchmarkEncryptReader_1MB            27             26             -3.70%
BenchmarkEncryptReader_1MB-32         27             26             -3.70%
BenchmarkDecryptReader_8KB            18             17             -5.56%
BenchmarkDecryptReader_8KB-32         18             17             -5.56%
BenchmarkDecryptReader_64KB           18             17             -5.56%
BenchmarkDecryptReader_64KB-32        18             17             -5.56%
BenchmarkDecryptReader_512KB          25             24             -4.00%
BenchmarkDecryptReader_512KB-32       25             24             -4.00%
BenchmarkDecryptReader_1MB            33             32             -3.03%
BenchmarkDecryptReader_1MB-32         33             32             -3.03%
BenchmarkDecryptReaderAt_8KB          24             22             -8.33%
BenchmarkDecryptReaderAt_8KB-32       24             22             -8.33%
BenchmarkDecryptReaderAt_64KB         24             22             -8.33%
BenchmarkDecryptReaderAt_64KB-32      24             22             -8.33%
BenchmarkDecryptReaderAt_512KB        29             27             -6.90%
BenchmarkDecryptReaderAt_512KB-32     29             27             -6.90%
BenchmarkDecryptReaderAt_1MB          37             35             -5.41%
BenchmarkDecryptReaderAt_1MB-32       37             35             -5.41%
BenchmarkEncryptWriter_8KB            12             11             -8.33%
BenchmarkEncryptWriter_8KB-32         12             11             -8.33%
BenchmarkEncryptWriter_64KB           12             11             -8.33%
BenchmarkEncryptWriter_64KB-32        12             11             -8.33%
BenchmarkEncryptWriter_512KB          19             18             -5.26%
BenchmarkEncryptWriter_512KB-32       19             18             -5.26%
BenchmarkEncryptWriter_1MB            27             26             -3.70%
BenchmarkEncryptWriter_1MB-32         27             26             -3.70%
BenchmarkDecryptWriter_8KB            14             13             -7.14%
BenchmarkDecryptWriter_8KB-32         14             13             -7.14%
BenchmarkDecryptWriter_64KB           14             13             -7.14%
BenchmarkDecryptWriter_64KB-32        14             13             -7.14%
BenchmarkDecryptWriter_512KB          21             20             -4.76%
BenchmarkDecryptWriter_512KB-32       21             20             -4.76%
BenchmarkDecryptWriter_1MB            29             28             -3.45%
BenchmarkDecryptWriter_1MB-32         29             28             -3.45%

benchmark                             old bytes     new bytes     delta
BenchmarkEncryptReader_8KB            74877         1144          -98.47%
BenchmarkEncryptReader_8KB-32         75206         1549          -97.94%
BenchmarkEncryptReader_64KB           74877         1144          -98.47%
BenchmarkEncryptReader_64KB-32        75646         1705          -97.75%
BenchmarkEncryptReader_512KB          74990         1256          -98.33%
BenchmarkEncryptReader_512KB-32       75902         1764          -97.68%
BenchmarkEncryptReader_1MB            75117         1384          -98.16%
BenchmarkEncryptReader_1MB-32         75868         1712          -97.74%
BenchmarkDecryptReader_8KB            75125         1392          -98.15%
BenchmarkDecryptReader_8KB-32         75171         1908          -97.46%
BenchmarkDecryptReader_64KB           75126         1392          -98.15%
BenchmarkDecryptReader_64KB-32        75182         2012          -97.32%
BenchmarkDecryptReader_512KB          75237         1504          -98.00%
BenchmarkDecryptReader_512KB-32       75347         1709          -97.73%
BenchmarkDecryptReader_1MB            75366         1633          -97.83%
BenchmarkDecryptReader_1MB-32         75505         1809          -97.60%
BenchmarkDecryptReaderAt_8KB          149123        1656          -98.89%
BenchmarkDecryptReaderAt_8KB-32       149729        2544          -98.30%
BenchmarkDecryptReaderAt_64KB         149126        1657          -98.89%
BenchmarkDecryptReaderAt_64KB-32      149804        2221          -98.52%
BenchmarkDecryptReaderAt_512KB        149255        1784          -98.80%
BenchmarkDecryptReaderAt_512KB-32     149511        1935          -98.71%
BenchmarkDecryptReaderAt_1MB          149542        2074          -98.61%
BenchmarkDecryptReaderAt_1MB-32       150141        2381          -98.41%
BenchmarkEncryptWriter_8KB            74843         1112          -98.51%
BenchmarkEncryptWriter_8KB-32         74854         1354          -98.19%
BenchmarkEncryptWriter_64KB           74883         1144          -98.47%
BenchmarkEncryptWriter_64KB-32        75237         1206          -98.40%
BenchmarkEncryptWriter_512KB          77223         3423          -95.57%
BenchmarkEncryptWriter_512KB-32       80640         6186          -92.33%
BenchmarkEncryptWriter_1MB            85695         11542         -86.53%
BenchmarkEncryptWriter_1MB-32         101491        25308         -75.06%
BenchmarkDecryptWriter_8KB            75002         1272          -98.30%
BenchmarkDecryptWriter_8KB-32         75029         1720          -97.71%
BenchmarkDecryptWriter_64KB           75004         1272          -98.30%
BenchmarkDecryptWriter_64KB-32        75028         1662          -97.78%
BenchmarkDecryptWriter_512KB          75175         1438          -98.09%
BenchmarkDecryptWriter_512KB-32       75304         1561          -97.93%
BenchmarkDecryptWriter_1MB            75468         1730          -97.71%
BenchmarkDecryptWriter_1MB-32         75857         2029          -97.33%
```
  • Loading branch information
klauspost committed May 6, 2021
1 parent 2535ed3 commit a9cfd77
Show file tree
Hide file tree
Showing 5 changed files with 227 additions and 104 deletions.
55 changes: 39 additions & 16 deletions reader-v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"errors"
"io"
"io/ioutil"
"sync"
)

type encReaderV10 struct {
Expand All @@ -28,6 +27,7 @@ type encReaderV10 struct {
buffer packageV10
offset int
payloadSize int
stateErr error
}

// encryptReaderV10 returns an io.Reader wrapping the given io.Reader.
Expand All @@ -40,12 +40,20 @@ func encryptReaderV10(src io.Reader, config *Config) (*encReaderV10, error) {
return &encReaderV10{
authEncV10: ae,
src: src,
buffer: make(packageV10, maxPackageSize),
buffer: packageBufferPool.Get().([]byte)[:maxPackageSize],
payloadSize: config.PayloadSize,
}, nil
}

func (r *encReaderV10) recycle() {
recyclePackageBufferPool(r.buffer)
r.buffer = nil
}

func (r *encReaderV10) Read(p []byte) (int, error) {
if r.stateErr != nil {
return 0, r.stateErr
}
var n int
if r.offset > 0 { // write the buffered package to p
remaining := r.buffer.Length() - r.offset // remaining encrypted bytes
Expand All @@ -61,6 +69,8 @@ func (r *encReaderV10) Read(p []byte) (int, error) {
for len(p) >= headerSize+r.payloadSize+tagSize {
nn, err := io.ReadFull(r.src, p[headerSize:headerSize+r.payloadSize]) // read plaintext from src
if err != nil && err != io.ErrUnexpectedEOF {
r.recycle()
r.stateErr = err
return n, err // return if reading from src fails or reached EOF
}
r.Seal(p, p[headerSize:headerSize+nn])
Expand All @@ -70,6 +80,8 @@ func (r *encReaderV10) Read(p []byte) (int, error) {
if len(p) > 0 {
nn, err := io.ReadFull(r.src, r.buffer[headerSize:headerSize+r.payloadSize]) // read plaintext from src
if err != nil && err != io.ErrUnexpectedEOF {
r.stateErr = err
r.recycle()
return n, err // return if reading from src fails or reached EOF
}
r.Seal(r.buffer, r.buffer[headerSize:headerSize+nn])
Expand All @@ -87,8 +99,9 @@ type decReaderV10 struct {
authDecV10
src io.Reader

buffer packageV10
offset int
buffer packageV10
offset int
stateErr error
}

// decryptReaderV10 returns an io.Reader wrapping the given io.Reader.
Expand All @@ -101,11 +114,19 @@ func decryptReaderV10(src io.Reader, config *Config) (*decReaderV10, error) {
return &decReaderV10{
authDecV10: ad,
src: src,
buffer: make(packageV10, maxPackageSize),
buffer: packageBufferPool.Get().([]byte)[:maxPackageSize],
}, nil
}

func (r *decReaderV10) recycle() {
recyclePackageBufferPool(r.buffer)
r.buffer = nil
}

func (r *decReaderV10) Read(p []byte) (n int, err error) {
if r.stateErr != nil {
return 0, r.stateErr
}
if r.offset > 0 { // write the buffered plaintext to p
payload := r.buffer.Payload()
remaining := len(payload) - r.offset // remaining plaintext bytes
Expand All @@ -120,21 +141,29 @@ func (r *decReaderV10) Read(p []byte) (n int, err error) {
}
for len(p) >= maxPayloadSize {
if err = r.readPackage(r.buffer); err != nil {
r.stateErr = err
r.recycle()
return n, err
}
length := len(r.buffer.Payload())
if err = r.Open(p[:length], r.buffer[:r.buffer.Length()]); err != nil { // notice: buffer.Length() may be smaller than len(buffer)
r.stateErr = err
r.recycle()
return n, err // decryption failed
}
p = p[length:]
n += length
}
if len(p) > 0 {
if err = r.readPackage(r.buffer); err != nil {
r.stateErr = err
r.recycle()
return n, err
}
payload := r.buffer.Payload()
if err = r.Open(payload, r.buffer[:r.buffer.Length()]); err != nil { // notice: buffer.Length() may be smaller than len(buffer)
r.stateErr = err
r.recycle()
return n, err // decryption failed
}
if len(payload) < len(p) {
Expand Down Expand Up @@ -170,8 +199,7 @@ func (r *decReaderV10) readPackage(dst packageV10) error {
type decReaderAtV10 struct {
src io.ReaderAt

ad authDecV10
bufPool sync.Pool
ad authDecV10
}

// decryptReaderAtV10 returns an io.ReaderAt wrapping the given io.ReaderAt.
Expand All @@ -185,12 +213,7 @@ func decryptReaderAtV10(src io.ReaderAt, config *Config) (*decReaderAtV10, error
ad: ad,
src: src,
}
r.bufPool = sync.Pool{
New: func() interface{} {
b := make([]byte, maxPackageSize)
return &b
},
}

return r, nil
}

Expand All @@ -204,12 +227,12 @@ func (r *decReaderAtV10) ReadAt(p []byte, offset int64) (n int, err error) {
return 0, errUnexpectedSize
}

buffer := r.bufPool.Get().(*[]byte)
defer r.bufPool.Put(buffer)
buffer := packageBufferPool.Get().([]byte)[:maxPackageSize]
defer recyclePackageBufferPool(buffer)
decReader := decReaderV10{
authDecV10: r.ad,
src: &sectionReader{r.src, t * maxPackageSize},
buffer: packageV10(*buffer),
buffer: packageV10(buffer),
offset: 0,
}
decReader.SeqNum = uint32(t)
Expand Down
70 changes: 55 additions & 15 deletions reader-v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,24 @@ type encReaderV20 struct {
buffer packageV20
offset int
lastByte byte
stateErr error

firstRead bool
}

var packageBufferPool = sync.Pool{New: func() interface{} { return make([]byte, maxPackageSize) }}

func recyclePackageBufferPool(b []byte) {
if cap(b) < maxPackageSize {
return
}
// Clear so we don't potentially leak info between callers
for i := range b {
b[i] = 0
}
packageBufferPool.Put(b)
}

// encryptReaderV20 returns an io.Reader wrapping the given io.Reader.
// The returned io.Reader encrypts everything it reads using DARE 2.0.
func encryptReaderV20(src io.Reader, config *Config) (*encReaderV20, error) {
Expand All @@ -42,12 +56,20 @@ func encryptReaderV20(src io.Reader, config *Config) (*encReaderV20, error) {
return &encReaderV20{
authEncV20: ae,
src: src,
buffer: make(packageV20, maxPackageSize),
buffer: packageBufferPool.Get().([]byte)[:maxPackageSize],
firstRead: true,
}, nil
}

func (r *encReaderV20) recycle() {
recyclePackageBufferPool(r.buffer)
r.buffer = nil
}

func (r *encReaderV20) Read(p []byte) (n int, err error) {
if r.stateErr != nil {
return 0, r.stateErr
}
if r.firstRead {
r.firstRead = false
_, err = io.ReadFull(r.src, r.buffer[headerSize:headerSize+1])
Expand All @@ -56,6 +78,8 @@ func (r *encReaderV20) Read(p []byte) (n int, err error) {
}
if err == io.EOF {
r.finalized = true
r.stateErr = io.EOF
r.recycle()
return 0, io.EOF
}
r.lastByte = r.buffer[headerSize]
Expand All @@ -72,6 +96,8 @@ func (r *encReaderV20) Read(p []byte) (n int, err error) {
r.offset = 0
}
if r.finalized {
r.stateErr = io.EOF
r.recycle()
return n, io.EOF
}
for len(p) >= maxPackageSize {
Expand All @@ -93,12 +119,16 @@ func (r *encReaderV20) Read(p []byte) (n int, err error) {
r.buffer[headerSize] = r.lastByte
nn, err := io.ReadFull(r.src, r.buffer[headerSize+1:headerSize+1+maxPayloadSize]) // try to read the max. payload
if err != nil && err != io.EOF && err != io.ErrUnexpectedEOF {
r.stateErr = err
r.recycle()
return n, err // failed to read from src
}
if err == io.EOF || err == io.ErrUnexpectedEOF { // read less than 64KB -> final package
r.SealFinal(r.buffer, r.buffer[headerSize:headerSize+1+nn])
if len(p) > r.buffer.Length() {
n += copy(p, r.buffer[:r.buffer.Length()])
r.stateErr = io.EOF
r.recycle()
return n, io.EOF
}
} else {
Expand All @@ -115,8 +145,9 @@ type decReaderV20 struct {
authDecV20
src io.Reader

buffer packageV20
offset int
stateErr error
buffer packageV20
offset int
}

// decryptReaderV20 returns an io.Reader wrapping the given io.Reader.
Expand All @@ -129,11 +160,19 @@ func decryptReaderV20(src io.Reader, config *Config) (*decReaderV20, error) {
return &decReaderV20{
authDecV20: ad,
src: src,
buffer: make(packageV20, maxPackageSize),
buffer: packageBufferPool.Get().([]byte)[:maxPackageSize],
}, nil
}

func (r *decReaderV20) recycle() {
recyclePackageBufferPool(r.buffer)
r.buffer = nil
}

func (r *decReaderV20) Read(p []byte) (n int, err error) {
if r.stateErr != nil {
return 0, r.stateErr
}
if r.offset > 0 { // write the buffered plaintext to p
remaining := len(r.buffer.Payload()) - r.offset
if len(p) < remaining {
Expand All @@ -151,9 +190,13 @@ func (r *decReaderV20) Read(p []byte) (n int, err error) {
return n, errUnexpectedEOF // reached EOF but not seen final package yet
}
if err != nil && err != io.ErrUnexpectedEOF {
r.recycle()
r.stateErr = err
return n, err // reading from src failed or reached EOF
}
if err = r.Open(p, r.buffer[:nn]); err != nil {
r.recycle()
r.stateErr = err
return n, err // decryption failed
}
p = p[len(r.buffer.Payload()):]
Expand All @@ -162,9 +205,13 @@ func (r *decReaderV20) Read(p []byte) (n int, err error) {
if len(p) > 0 {
nn, err := io.ReadFull(r.src, r.buffer)
if err == io.EOF && !r.finalized {
r.recycle()
r.stateErr = errUnexpectedEOF
return n, errUnexpectedEOF // reached EOF but not seen final package yet
}
if err != nil && err != io.ErrUnexpectedEOF {
r.recycle()
r.stateErr = err
return n, err // reading from src failed or reached EOF
}
if err = r.Open(r.buffer[headerSize:], r.buffer[:nn]); err != nil {
Expand Down Expand Up @@ -217,8 +264,7 @@ func decryptBufferV20(dst, src []byte, config *Config) ([]byte, error) {
type decReaderAtV20 struct {
src io.ReaderAt

ad authDecV20
bufPool sync.Pool
ad authDecV20
}

// decryptReaderAtV20 returns an io.ReaderAt wrapping the given io.ReaderAt.
Expand All @@ -232,12 +278,7 @@ func decryptReaderAtV20(src io.ReaderAt, config *Config) (*decReaderAtV20, error
ad: ad,
src: src,
}
r.bufPool = sync.Pool{
New: func() interface{} {
b := make([]byte, maxPackageSize)
return &b
},
}

return r, nil
}

Expand All @@ -251,14 +292,13 @@ func (r *decReaderAtV20) ReadAt(p []byte, offset int64) (n int, err error) {
return 0, errUnexpectedSize
}

buffer := r.bufPool.Get().(*[]byte)
defer r.bufPool.Put(buffer)
decReader := decReaderV20{
authDecV20: r.ad,
src: &sectionReader{r.src, t * maxPackageSize},
buffer: packageV20(*buffer),
buffer: packageBufferPool.Get().([]byte)[:maxPackageSize],
offset: 0,
}
defer decReader.recycle()
decReader.SeqNum = uint32(t)
if k := offset % int64(maxPayloadSize); k > 0 {
if _, err := io.CopyN(ioutil.Discard, &decReader, k); err != nil {
Expand Down
Loading

0 comments on commit a9cfd77

Please sign in to comment.