Add Binary Allocator #14201
Add Binary Allocator #14201
Conversation
| private int sampleCount; | ||
| private final int weight; | ||
| private boolean isOld; // Enable to have enough historical data | ||
| private final int OLD_THRESHOLD = 100; |
|
|
||
| import static java.lang.Math.max; | ||
|
|
||
| public class AdaptiveWeightedAverage { |
| sampleCount = 0; | ||
| } | ||
|
|
||
| public void sample(int newSample) { |
There was a problem hiding this comment.
This does not seem to be writing friendly, and it feels like adaptiveWeightedAverage is a write more read less structure, should not be implemented this way.
In addition whether we use some mature EMA class such as "com.codahale.metrics.EWMA, don't have to write yourself?
There was a problem hiding this comment.
The EWMA implementation in com.codahale.metrics introduces extra work to improve concurrent performance, which wastes additional memory and reduces update performance. We don't need to consider concurrency issues here.
BTW, the AdaptiveWeightedAverage is modified from JDK17 source codes. However, I made some modifications after discovering that memory allocation and deallocation occurs in cycles, such as before and after a request completes. Therefore, the active memory volume fluctuates significantly, so we calculate a moving average based on the peak active memory within each evictor cycle.
| /** The default value for {@code evictorShutdownTimeout} configuration attribute. */ | ||
| private long evictorShutdownTimeoutMillis = 10L * 1000L; | ||
|
|
||
| /** The default value for {@code evictorShutdownTimeout} configuration attribute. */ |
There was a problem hiding this comment.
Please describe the meaning of these parameters in the Javadoc. The way it is written now is worse than not writing it at all...
| private int[] sizeIdx2sizeTab; | ||
| final int lookupMaxSize; | ||
|
|
||
| final int LOG2_QUANTUM; |
There was a problem hiding this comment.
why not private？
There was a problem hiding this comment.
Do not capitalize variables that are not final static...
|
|
||
| public PooledBinary allocateBinary(int reqCapacity) { | ||
| if (reqCapacity < allocatorConfig.minAllocateSize | ||
| | reqCapacity > allocatorConfig.maxAllocateSize) { |
| state.set(BinaryAllocatorState.OPEN); | ||
| evictor = | ||
| new GCEvictor( | ||
| "binary-allocator-gc-evictor", allocatorConfig.getDurationEvictorShutdownTimeout()); |
There was a problem hiding this comment.
do no use magic string
| } | ||
|
|
||
| public void close(boolean needReopen) { | ||
| if (needReopen) state.set(BinaryAllocatorState.TMP_CLOSE); |
|
|
||
| package org.apache.iotdb.commons.utils.binaryallocator; | ||
|
|
||
| public enum BinaryAllocatorState { |
There was a problem hiding this comment.
Describe the transition relationship between these states, and what conditions trigger the transition
| AllocatorConfig config = new AllocatorConfig(); | ||
| config.arenaNum = 1; | ||
| BinaryAllocator binaryAllocator = new BinaryAllocator(config); | ||
| PooledBinary binary = binaryAllocator.allocateBinary(256); |
There was a problem hiding this comment.
use Odd number like 255 is better to test jvm allocation？
Description
Add Binary Allocator for object pooling and reuse of large Binary objects to reduce GC overhead.
Features
Details
Next Steps
Integrate Binary Allocator into IoTDB's writing execution.