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

Max out the protobuf file read size limit #1756

Merged
merged 1 commit into from
Jan 20, 2015

Conversation

jeffdonahue
Copy link
Contributor

This sets the max possible size of protobuf reads to 2 GB minus 1 byte (= 2^31 - 1), the largest int value. (If we want to start supporting nets with total param size >= 2 GB this we'll have to come up with a new solution. For example, we could store each parameter in a separate binary proto file, or switch to HDF5 for parameter serialization.)

@longjon
Copy link
Contributor

longjon commented Jan 20, 2015

Yes, I've run into this before. Perhaps we should make this less mysterious by using INT_MAX?

@jeffdonahue
Copy link
Contributor Author

yup, good idea, fixed

@@ -17,6 +17,8 @@
#include "caffe/proto/caffe.pb.h"
#include "caffe/util/io.hpp"

#define kProtoReadBytesLimit INT_MAX // Max size of 2 GB minus 1 byte.
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is C++, why not const int?

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

longjon added a commit that referenced this pull request Jan 20, 2015
Max out the protobuf file read size limit
@longjon longjon merged commit 350d880 into BVLC:dev Jan 20, 2015
@longjon
Copy link
Contributor

longjon commented Jan 20, 2015

Look good, merged. It would be nice to also break out the other mysterious constant at some point...

@jeffdonahue jeffdonahue deleted the max-total-bytes-limit branch January 20, 2015 01:05
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