I really like it. I pushed a version of this that gets to the heart of the idea. I need to move on to the messages themselves as I have a hard stop approaching me, but I am supportive if you want to keep refining it.
Ideally, we merge and you make any changes you feel are necessary and I make second PR for rolling this out to the messages. -David > On Apr 26, 2022, at 12:02 AM, Romain Manni-Bucau <[email protected]> > wrote: > > Hey David, > > Just got an idea we can maybe refine to merge both points. > I'll try to draft it high level (understand a Writer can be an OutputStream > or the opposite in your impl) so don't get too picky about the details > please ;). > > High level the idea is to make JsonGeneratorFactoryImpl to be able to > handle an outputstream or writer which would provide its buffer size. > > Here are the idea steps I can envision: > > 1. Create a new interface in johnzon-core `public interface > MetadataProvider { int bufferSize(); }` > 2. Create an OutputStream implementation of MetadataProvider - let's call > it MetadataProviderOutputStream for this list - if it helps it can be a > MetadataProviderSnippetOutputStream directly which would just be a subclass > of SnippetOutputStream, think it would make it simpler. > 3. Create a Supplier<OutputStream> (factory) in MapperBuilder - where we > create the Snippet instance - which will basically test the class name of > the generator factory - no reflection there please to stay compatible with > all our downstream consumers without changes - and if it > is org.apache.johnzon.core.JsonGeneratorFactoryImpl then it will create a > MetadataProviderOutputStream else a plain OutputStream and the feature will > be disabled (for now at least) - note that it can need a small indirection > like a MetadataProviderOutputStreamSupplier to not trigger the load of the > MetadataProvider on MapperBuilder class init/verification. This will also > use the snippet max size indeed. Here we want to ensure we can run without > johnzon-core as JSON-P impl. > 4. Pass the supplier created at 3 to the Snippet class next to the > generator factory > 5. Change JsonGeneratorFactoryImpl#createGenerator(OutputStream) to test if > it is a MetadataProvider instance to handle it (small trick there is to try > to cache the snippet buffer to keep reusing the buffer caching - and only > when needed): > > public class JsonGeneratorFactoryImpl extends AbstractJsonFactory > implements JsonGeneratorFactory { > public static final String GENERATOR_BUFFER_LENGTH = > "org.apache.johnzon.default-char-buffer-generator"; > public static final int DEFAULT_GENERATOR_BUFFER_LENGTH = > Integer.getInteger(GENERATOR_BUFFER_LENGTH, 64 * 1024); //64k > > static final Collection<String> SUPPORTED_CONFIG_KEYS = asList( > JsonGenerator.PRETTY_PRINTING, GENERATOR_BUFFER_LENGTH, > BUFFER_STRATEGY > ); > //key caching currently disabled > private final boolean pretty; > private final Buffer buffer; > private volatile Buffer firstCustomBuffer; > > public JsonGeneratorFactoryImpl(final Map<String, ?> config) { > super(config, SUPPORTED_CONFIG_KEYS, null); > this.pretty = getBool(JsonGenerator.PRETTY_PRINTING, false); > > final int bufferSize = getInt(GENERATOR_BUFFER_LENGTH, > DEFAULT_GENERATOR_BUFFER_LENGTH); > if (bufferSize <= 0) { > throw new IllegalArgumentException("buffer length must be > greater than zero"); > } > this.buffer = new > Buffer(getBufferProvider().newCharProvider(bufferSize), bufferSize); > } > > @Override > public JsonGenerator createGenerator(final Writer writer) { > return new JsonGeneratorImpl(writer, buffer.provider, pretty); > } > > @Override > public JsonGenerator createGenerator(final OutputStream out) { > if (MetadataProvider.class.isInstance(out)) { // mainly for Snippet > handling since generators don't have properties > final int bufferSize = > MetadataProvider.class.cast(out).bufferSize(); > if (buffer.size != bufferSize) { > // most of the time it will be a single aside buffer so > keep its factory to reuse the provider cache > if (firstCustomBuffer == null) { > synchronized (this) { > if (firstCustomBuffer == null) { > firstCustomBuffer = new > Buffer(getBufferProvider().newCharProvider(bufferSize), bufferSize); > } > } > } > Buffer customBuffer = firstCustomBuffer; > if (customBuffer.size != bufferSize) { // don't use the > cache, shouldn't occur often > return new JsonGeneratorImpl(out, > getBufferProvider().newCharProvider(bufferSize), pretty); > } > return new JsonGeneratorImpl(out, customBuffer.provider, > pretty); > } // else just reuse the original buffer since it matches and > leverages its cache > } > return new JsonGeneratorImpl(out, buffer.provider, pretty); > } > > @Override > public JsonGenerator createGenerator(final OutputStream out, final > Charset charset) { > return new JsonGeneratorImpl(out,charset, buffer.provider, pretty); > } > > @Override > public Map<String, ?> getConfigInUse() { > return Collections.unmodifiableMap(internalConfig); > } > > private static final class Buffer { > private final BufferStrategy.BufferProvider<char[]> provider; > private final int size; > > private Buffer(final BufferStrategy.BufferProvider<char[]> > bufferProvider, final int bufferSize) { > this.provider = bufferProvider; > this.size = bufferSize; > } > } > } > > 6. you use the outputstream supplier in Snippet and done > > Wdyt? > > Romain Manni-Bucau > @rmannibucau <https://twitter.com/rmannibucau> | Blog > <https://rmannibucau.metawerx.net/> | Old Blog > <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> | > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book > <https://www.packtpub.com/application-development/java-ee-8-high-performance> > > > Le mar. 26 avr. 2022 à 07:05, Romain Manni-Bucau <[email protected]> a > écrit : > >> >> >> Le mar. 26 avr. 2022 à 00:44, David Blevins <[email protected]> a >> écrit : >> >>> Since we're now getting the snippet max length via the config, could we >>> construct the JsonGeneratorImpl directly and pass in a buffer of the >>> correct size? >>> >> >> From the JsonProvider we can - mapper must not depend on johnzon-core - at >> the condition to promote the other properties too AND not do it if user >> provided a custom generator factory which would just be reused in this >> case, so not sure it solves the issue. >> >> >>> -David >>> >>> >>>> On Apr 25, 2022, at 2:19 PM, Romain Manni-Bucau <[email protected]> >>> wrote: >>>> >>>> No worries, got it. >>>> >>>> One issue copying the generator is that it will still miss the flush for >>>> all small objects and for all other cases the heuristic + a simple >>>> truncation if needed would work with almost no other downsides than not >>>> being exact but goal was to limit the overflow/cpu/mem so all good IMHO. >>>> >>>> Le lun. 25 avr. 2022 à 22:57, David Blevins <[email protected]> a >>>> écrit : >>>> >>>>>> On Apr 25, 2022, at 10:47 AM, David Blevins <[email protected]> >>>>> wrote: >>>>>> >>>>>> The trick with that is there is yet another buffer being held >>> internally >>>>> by ObjectOutputStream and it recreates the issue. >>>>> >>>>> Just read this again -- this was supposed to be "OutputStreamWriter" >>> not >>>>> ObjectOutputStream. Too many years of EJB :) >>>>> >>>>> >>>>> -David >>>>> >>>>> >>> >>>
smime.p7s
Description: S/MIME cryptographic signature
