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
>> >>
>> >>
>>
>>
