[ https://issues.apache.org/jira/browse/FLINK-8178?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16292517#comment-16292517 ]
ASF GitHub Bot commented on FLINK-8178: --------------------------------------- Github user pnowojski commented on a diff in the pull request: https://github.com/apache/flink/pull/5105#discussion_r157154019 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/io/network/buffer/BufferBuilder.java --- @@ -0,0 +1,80 @@ +/* + * 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.flink.runtime.io.network.buffer; + +import org.apache.flink.core.memory.MemorySegment; + +import javax.annotation.concurrent.NotThreadSafe; + +import java.nio.ByteBuffer; + +import static org.apache.flink.util.Preconditions.checkNotNull; +import static org.apache.flink.util.Preconditions.checkState; + +/** + * Not thread safe class for filling in the initial content of the {@link Buffer}. Once writing to the builder + * is complete, {@link Buffer} instance can be built and shared across multiple threads. + */ +@NotThreadSafe +public class BufferBuilder { + private final MemorySegment memorySegment; + + private final BufferRecycler recycler; + + private int position = 0; + + private boolean built = false; --- End diff -- We could, by what's the benefit of that? You would end up with ugly cryptic checkstates: `checkState(position >= 0)` and cryptic setting `position = -1` after building the buffer. You could wrap them into some helper functions like `checkState(!isBuilt())` and `markPositionBuilt()` but I think that it's uglier and harder to understand then the explicit check variable. > Introduce not threadsafe write only BufferBuilder > ------------------------------------------------- > > Key: FLINK-8178 > URL: https://issues.apache.org/jira/browse/FLINK-8178 > Project: Flink > Issue Type: Improvement > Components: Network > Reporter: Piotr Nowojski > Assignee: Piotr Nowojski > Fix For: 1.5.0 > > > While Buffer class is used in multithreaded context it requires > synchronisation. Now it is miss-leading/unclear and suggesting that > RecordSerializer should take into account synchronisation of the Buffer > that's holding. With NotThreadSafe BufferBuilder there would be clear > separation between single-threaded writing/creating a BufferBuilder and > multithreaded Buffer handling/retaining/recycling. -- This message was sent by Atlassian JIRA (v6.4.14#64029)