[core] CoreOptions.fileFormat is cpu expensive, because FileFormat initalization is costly. As much as we can, reduce the rate. #4782
Conversation
…italizing is costly. As much as we can, reduce the rate
| @@ -88,7 +103,7 @@ public static FileFormat fromIdentifier(String identifier, Options options) { | |||
|
|
|||
| /** Create a {@link FileFormat} from format identifier and format options. */ | |||
| public static FileFormat fromIdentifier(String identifier, FormatContext context) { | |||
| return fromIdentifier(identifier, context, FileFormat.class.getClassLoader()) | |||
| return getFileFormatFromLoadedCache(identifier, context) | |||
There was a problem hiding this comment.
Use org.apache.paimon.factories.FactoryUtil
| @@ -101,11 +104,13 @@ protected AbstractFileStore( | |||
| this.writeManifestCache = | |||
| SegmentsCache.create( | |||
| options.pageSize(), options.writeManifestCache(), Long.MAX_VALUE); | |||
| this.fileFormat = options.fileFormat(); | |||
| this.pathFactory = pathFactory(fileFormat.getFormatIdentifier()); | |||
There was a problem hiding this comment.
Don't create path factory here... It should be created one by one
| import java.util.ServiceLoader; | ||
|
|
||
| /** Base Factory Util. */ | ||
| public class BaseFactoryUtil { |
There was a problem hiding this comment.
Do not introduce this method, it has no abstraction... Just put static method into FactoryUtil.
| @@ -120,6 +121,7 @@ public AppendOnlyFileStoreWrite newWrite( | |||
| commitUser, | |||
| rowType, | |||
| partitionType, | |||
| options.fileFormat(), | |||
There was a problem hiding this comment.
Why we need to modify this?
|
extends Factory is more batter |
…pache#4782) (cherry picked from commit 2e57c59)
|
This pr is from my pr #4497 , You spared me a private discussion and plagiarized my PR. Without my consent, you quickly merged the PR and then closed my original PR at the same time. Why do this? Do you follow the Apache open source spirit? stop your mistakes please @JingsongLi @leaves12138 |
|
@ranxianglei |
|
@ranxianglei Even in #4813 , you have never provided a better version or convincing reasons. |
Purpose
Just don't call
options.fileformatas much as you want.Tests
API and Format
Documentation