Also had an idea as I was trying to go to bed. The thought occurred to me that we only need to call flush before calling snippetOutputStream.terminate(). So I reworked the code to do that. We also had some calls to terminate() that we didn't need because they were already getting called in the loops that proceeded them.
I'll read what you wrote below, but can you take a look and let me know what
you think?
-David
> On Apr 26, 2022, at 12:02 AM, Romain Manni-Bucau <rmannibu...@gmail.com>
> 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 <rmannibu...@gmail.com> a
> écrit :
>
>>
>>
>> Le mar. 26 avr. 2022 à 00:44, David Blevins <david.blev...@gmail.com> 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 <rmannibu...@gmail.com>
>>> 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 <david.blev...@gmail.com> a
>>>> écrit :
>>>>
>>>>>> On Apr 25, 2022, at 10:47 AM, David Blevins <david.blev...@gmail.com>
>>>>> 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
