timoninmaxim commented on code in PR #11118: URL: https://github.com/apache/ignite/pull/11118#discussion_r1471115198
########## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/dump/BufferedFileIO.java: ########## @@ -0,0 +1,128 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.processors.cache.persistence.snapshot.dump; + +import java.io.IOException; +import java.nio.ByteBuffer; +import org.apache.ignite.internal.processors.cache.persistence.file.FileIO; +import org.apache.ignite.internal.processors.cache.persistence.file.FileIODecorator; +import org.apache.ignite.internal.util.typedef.internal.A; + +/** + * The class implements a {@link FileIO} that allows to write bytes to the underlying {@link FileIO} without necessarily + * causing a call to the underlying system for each byte written. + * It doesn't support reading or random access. It is not designed for writing concurrently from several threads. + */ +public class BufferedFileIO extends FileIODecorator { + /** */ + private ByteBuffer buf; + + /** */ + public BufferedFileIO(FileIO fileIO, int bufSz) { + super(fileIO); + + A.ensure(fileIO != null, "fileIO must not be null"); + A.ensure(bufSz > 0, "bufSz must be positive"); + + buf = ByteBuffer.allocateDirect(bufSz); + } + + /** {@inheritDoc} */ + @Override public int write(ByteBuffer srcBuf) throws IOException { + if (buf == null) + throw new IOException("FileIO closed"); + + int bytesCnt = srcBuf.remaining(); + + int limit = srcBuf.limit(); + + while (srcBuf.hasRemaining()) { + if (srcBuf.remaining() > buf.remaining()) + srcBuf.limit(srcBuf.position() + buf.remaining()); + + buf.put(srcBuf); + + if (!buf.hasRemaining()) + flush(); + + srcBuf.limit(limit); + } + + return bytesCnt; + } + + /** {@inheritDoc} */ + @Override public int write(byte[] srcBuf, int off, int len) throws IOException { + if (buf == null) + throw new IOException("FileIO closed"); + + if ((len | off) < 0 || len > srcBuf.length - off) + throw new IndexOutOfBoundsException("Range [" + off + ", " + off + " + " + len + ") out of bounds for length " + srcBuf.length); + + int p = off; + + while (p < off + len) { + int bytesCnt = Math.min(buf.remaining(), off + len - p); + + buf.put(srcBuf, p, bytesCnt); + + if (!buf.hasRemaining()) + flush(); + + p += bytesCnt; + } + + return len; + } + + /** + * Writes one byte. + * @throws IOException If some I/O error occurs. + */ + public void write(byte b) throws IOException { + if (buf == null) + throw new IOException("FileIO closed"); + + buf.put(b); + + if (!buf.hasRemaining()) + flush(); + } + + /** */ + private void flush() throws IOException { + if (buf == null) + return; Review Comment: Why other methods throw IOException when `buf == null` and this method doesn't? ########## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/dump/BufferedFileIO.java: ########## @@ -0,0 +1,128 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.processors.cache.persistence.snapshot.dump; + +import java.io.IOException; +import java.nio.ByteBuffer; +import org.apache.ignite.internal.processors.cache.persistence.file.FileIO; +import org.apache.ignite.internal.processors.cache.persistence.file.FileIODecorator; +import org.apache.ignite.internal.util.typedef.internal.A; + +/** + * The class implements a {@link FileIO} that allows to write bytes to the underlying {@link FileIO} without necessarily Review Comment: Let's mention that this IO writes to underlying storage whole buffer and it reduces amount of system calls. ########## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/dump/BufferedFileIOFactory.java: ########## @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.processors.cache.persistence.snapshot.dump; + +import java.io.File; +import java.io.IOException; +import java.nio.file.OpenOption; +import org.apache.ignite.internal.processors.cache.persistence.file.FileIO; +import org.apache.ignite.internal.processors.cache.persistence.file.FileIOFactory; + +/** + * File I/O factory which provides {@link BufferedFileIO} implementation of FileIO. + */ +public class BufferedFileIOFactory implements FileIOFactory { + /** */ + private static final long serialVersionUID = 0L; + + /** */ + private static final int DEFAULT_BLOCK_SIZE = 4096; + + /** */ + private final FileIOFactory factory; + + /** */ + public BufferedFileIOFactory(FileIOFactory factory) { + this.factory = factory; + } + + /** {@inheritDoc} */ + @Override public BufferedFileIO create(File file, OpenOption... modes) throws IOException { + FileIO io = factory.create(file, modes); + + int blockSize = io.getFileSystemBlockSize(); + + if (blockSize == -1) Review Comment: `blockSize < 0`, because javadocs says about "negative value", not for "-1". ########## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/dump/BufferedFileIO.java: ########## @@ -0,0 +1,128 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.processors.cache.persistence.snapshot.dump; + +import java.io.IOException; +import java.nio.ByteBuffer; +import org.apache.ignite.internal.processors.cache.persistence.file.FileIO; +import org.apache.ignite.internal.processors.cache.persistence.file.FileIODecorator; +import org.apache.ignite.internal.util.typedef.internal.A; + +/** + * The class implements a {@link FileIO} that allows to write bytes to the underlying {@link FileIO} without necessarily + * causing a call to the underlying system for each byte written. + * It doesn't support reading or random access. It is not designed for writing concurrently from several threads. + */ +public class BufferedFileIO extends FileIODecorator { + /** */ + private ByteBuffer buf; + + /** */ + public BufferedFileIO(FileIO fileIO, int bufSz) { + super(fileIO); + + A.ensure(fileIO != null, "fileIO must not be null"); + A.ensure(bufSz > 0, "bufSz must be positive"); + + buf = ByteBuffer.allocateDirect(bufSz); + } + + /** {@inheritDoc} */ + @Override public int write(ByteBuffer srcBuf) throws IOException { + if (buf == null) + throw new IOException("FileIO closed"); + + int bytesCnt = srcBuf.remaining(); + + int limit = srcBuf.limit(); + + while (srcBuf.hasRemaining()) { + if (srcBuf.remaining() > buf.remaining()) + srcBuf.limit(srcBuf.position() + buf.remaining()); + + buf.put(srcBuf); + + if (!buf.hasRemaining()) + flush(); + + srcBuf.limit(limit); + } + + return bytesCnt; + } + + /** {@inheritDoc} */ + @Override public int write(byte[] srcBuf, int off, int len) throws IOException { Review Comment: Is it possible to unify this 2 `write` methods? With `buf.array()`? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
