Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

use bytes.Buffer copy instead of make slice #2 #3

Closed
wants to merge 1 commit into from

Conversation

u5surf
Copy link

@u5surf u5surf commented Nov 19, 2018

#2
I fix to be using io.Copy from reader stream to byte.Buffer instead of making slice object.
And I would like to indicate memory usage performance. Let me how to measure the performance which you tested.

@alldroll
Copy link
Owner

For measure the performance you can use built in go prof tools or something like this

/usr/bin/time -lp (or -v)  ./dump data.cdb > /dev/null

data.cdb you can create by yourself with any csv file (with 2 columns) or use already created cdb file for instance https://github.com/alldroll/suggest/blob/master/testdata/db/cars.cdb

log.Fatal(err)
}
keyReader, _ := record.Key()
keyBuf := new(bytes.Buffer)
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think that this approach could help to optimize memory usage. Every loop step you allocate a bytes.Buffer object and io.Copy also allocates []byte. I think such code as

  var (
         key   = []byte{} // better to allocate with some capacity
         value = []byte{}
  )
  ....
  loop step
  ....

  if len(key) < int(keySize) {
       key = make([]byte, keySize*2)
  } // otherwise we save memory for key reading

  // the same for value reading
  ...
  
  err = csvWriter.Write([]string{string(key[:keySize]), string(value[:valSize])})

could help in the issue.

}
valReader, _ := record.Value()
valBuf := new(bytes.Buffer)
io.Copy(valBuf, valReader)
Copy link
Owner

Choose a reason for hiding this comment

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

It's a bad practice to ignore errors

@alldroll alldroll closed this Aug 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants