This repository has been archived by the owner. It is now read-only.

RecordReader.resetSupported() and concatenating record reader #469

Merged
merged 4 commits into from Nov 30, 2017

Conversation

Projects
None yet
2 participants
@AlexDBlack
Copy link
Member

AlexDBlack commented Nov 29, 2017

Adds RecordReader.resetSupported(), and ConcatenatingRecordReader

See also:
deeplearning4j/deeplearning4j#4345
deeplearning4j/deeplearning4j#4339

@AlexDBlack AlexDBlack changed the title Ab reset RecordReader.resetSupported() and concatenating record reader Nov 29, 2017

@Override
public boolean resetSupported() {
if( is != null){
return is.markSupported();

This comment has been minimized.

@saudet

saudet Nov 30, 2017

Member

When markSupported() == true, wouldn't we need to call mark() somewhere at the beginning for this to be valid?

This comment has been minimized.

@AlexDBlack

AlexDBlack Nov 30, 2017

Member

Good question, and you may be right. I should probably add a test for that...

@AlexDBlack

This comment has been minimized.

Copy link
Member

AlexDBlack commented Nov 30, 2017

I have decided to disallow resetting on all streams... re-reading the javadoc for mark: https://docs.oracle.com/javase/7/docs/api/java/io/InputStream.html#mark(int)

The general contract of mark is that, if the method markSupported returns true, the stream somehow remembers all the bytes read after the call to mark and stands ready to supply those same bytes again if and whenever the method reset is called. However, the stream is not required to remember any data at all if more than readlimit bytes are read from the stream before reset is called.

In other words, we have to specify a read limit value... In some cases, this might result in an internal storage buffer being used.
If the readLimit value is too small, reset isn't possible. If it's too large, we could use an unreasonable amount of memory (depending on the underlying stream implementation). Consequently, there's no single readLimit value that will work in all cases...

@AlexDBlack AlexDBlack merged commit 3bfe671 into master Nov 30, 2017

@AlexDBlack AlexDBlack deleted the ab_reset branch Nov 30, 2017

@saudet

This comment has been minimized.

Copy link
Member

saudet commented Nov 30, 2017

Sounds good to me!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.