Skip to content

Replace uses of POSIX open/close#3444

Open
eelstork wants to merge 1 commit intoBVLC:masterfrom
BonsaiAI:avoid-posix-io
Open

Replace uses of POSIX open/close#3444
eelstork wants to merge 1 commit intoBVLC:masterfrom
BonsaiAI:avoid-posix-io

Conversation

@eelstork
Copy link
Contributor

Protobuf related i/o in Caffe relies on POSIX open/close. These functions are not cross-platform.
If performance concerns are not critical, the proposed solution (kindly suggested by @willyd and @keenbrowne) may be adequate.

Previously, ReadXXX/ReadXXXOrDie, WriteXXX/WriteXXXOrDie variants were available; fail-slow variants had no uses in higher level APIs, possibly unsafe uses in tests and examples.

@ajtulloch
Copy link
Contributor

  • What's the perf change for loading a nontrivial model (say, a hundred MB) with this diff?
  • Having the non-CHECK'ing APIs are useful for cases where you're embedding Caffe in a larger process, and a failure to read a file doesn't need (and shouldn't) take down the whole process. I'd be in favor of not removing them.

@eelstork
Copy link
Contributor Author

@ajtulloch a performance dip is expected according to IstreamInputStream docs; I don't mind running a couple of tests if interested parties perceive that this is important - might you contribute data?

Although I agree that error handling should be customisable, the current implementation falls short. For sake, have a look at ReadProtoFromTextFile. An assertive check is carried out regardless, with the returned flag only covering parsing correctness. Is this expected behaviour?

Raising standard or custom i/o exceptions may be appropriate.

@willyd
Copy link
Contributor

willyd commented Dec 14, 2015

+1 for this as it will help many of us maintaining Windows forks of Caffe.

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.

3 participants