search

amazon-ion / ion-java

Java streaming parser/serializer for Ion.
https://amazon-ion.github.io/ion-docs/
Apache License 2.0
866 stars 110 forks source link

benchmark to see if PooledBlockAllocator can be replaced by the Basic one #225

Open raganhan opened 5 years ago

raganhan commented 5 years ago

With modern JVMs this may not be needed anymore we must benchmark both to verify and remove if we can

https://github.com/amzn/ion-java/blob/7992a822ba85fb538b3dd872f1a86b4d0e80906a/src/software/amazon/ion/impl/bin/PrivateIonManagedBinaryWriterBuilder.java#L48

develar commented 5 years ago

Currently, no API to manage PooledBlockAllocator. And it leads to memory leak, if you decided to cache IonBinaryWriterBuilder (like IonBinaryWriterBuilder.standard().immutable()).

image

Workaround — use _Private_IonManagedBinaryWriterBuilder directly: 

  val binaryWriterBuilder = _Private_IonManagedBinaryWriterBuilder
    .create(_Private_IonManagedBinaryWriterBuilder.AllocatorMode.BASIC)
    .withPaddedLengthPreallocation(0)
  binaryWriterBuilder
develar commented 5 years ago

In my fork I added ability to provide own implementation of BlockAllocatorProvider — https://github.com/JetBrains/ion-java/commit/cbe2b4914f902067d4e53ee492de0cb75d0a327e Later I will prepare PR. But now I see what's the problem — no API to release created BlockAllocator (created in vendAllocator) after use of writer. As I guess from javadoc:

Builders generally bind to an allocation pool as defined by BlockAllocatorProvider, so applications should reuse them as much as possible.

it is by design. In my case I will use my own BlockAllocatorProvider that will have such API to cleanup. I doubt that I will send PR about such thread-local block allocator provider, because my requirements is quite specific and I doubt that generic solution is applicable here (in any case, TIntObjectHashMap will be not possible to use in amazon ion code :)).

Yes — for modern JVM such long-running pool is not required. For example, after performance tests we got rid of such solutions in IntelliJ Platform lib (e.g. StringBuilderSpinAllocator). There are some exclusions, of course, as Netty uses own byte buffer pool (heap or native), but for serialiser it is overkill in most cases, I guess (thread-local (per session) still makes sense).

dlurton commented 5 years ago

@develar, we are are looking forward to the PR you mention to allow custom implementations of BlockAllocatorProvider. Thanks!

tgregg commented 5 years ago

Additionally, here's the issue to track exposing _Private_IonManagedBinaryWriterBuilder's configuration through IonBinaryWriterBuilder: #251

dlurton commented 5 years ago

@develar Is there anything we can do to help move this along? Thanks.