[MGNLACTIVATION-134] Update org.apache.commons:commons-collections4 to version 4.1 Created: 16/Aug/16  Updated: 09/Feb/17  Resolved: 25/Aug/16

Status: Closed
Project: Activation
Component/s: None
Affects Version/s: None
Fix Version/s: 5.5

Type: Improvement Priority: Neutral
Reporter: Cedric Reichenbach Assignee: Cedric Reichenbach
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Date of First Response:
Sprint: Basel 58
Story Points: 0

 Description   

In preparation for the library update of org.apache.commons:commons-collections4 to version 4.1, we add import to the dependency management section of magnolia-project so that the version of commons-collections4 is only managed in one place. Depending on the module this update includes removal of already existing deps in depMan section or addition of the dep to modules themselves. We also make sure that the changes build accordingly, as the actual update will happen upstream in magnolia-project.



 Comments   
Comment by Cedric Reichenbach [ 16/Aug/16 ]

TBD:

In commons-collections4, Buffers were replaced by Java's Queues.

In our MemoryActivationStorage, there are instances created like BufferUtils.synchronizedBuffer(new CircularFifoBuffer(500)), i.e. synchronized circular FIFO buffers/queues. There's a class CircularFifoQueue in commons4, SynchronizedBuffer has been dropped, referring to "corresponding *BlockingQueue classes". However, there seems to be no such class either doing the same as a circular fifo queue or being a synchronizing wrapper for other queues.

How should we handle this? Ideas:

  • Implement our own synchronizing queue wrapper, basically the same as SynchronizedBuffer did, just for Queues.
  • Check if we actually need a circular FIFO queue or if it could be replaced by an existing *BlockingQueue class.
  • Keep the commons (3) dependency for this module.
Comment by Jan Haderka [ 17/Aug/16 ]

Or perhaps there's yet another option:

  • replace buffer with MinMaxPriorityQueue which on it's own is also not thread safe, but guava fortunately provides wrapper for thread safety in form of Queues.synchronizedQueue(Queue q) call. Since we use guava elsewhere already it should be ok to use the library instead of commons. Not sure about performance comparison of the two tho.
Comment by Cedric Reichenbach [ 17/Aug/16 ]

Ah, it seems like Queues#synchronizedQueue is indeed the equivalent of BufferUtils#synchronizedBuffer, i.e. synchronizes every access to the contained queue/buffer. However, a more precise match in Guava for CircularFifoBuffer would probably be EvictingQueue, or, since the sync wrapper works for any Queue, even Java's own Apache's new CircularFifoQueue.

Comment by Cedric Reichenbach [ 18/Aug/16 ]

As suggested by ejervidalo, I made a simple performance comparison between

1. CircularFifoBuffer wrapped in a SynchronizedBuffer, both from commons-collections 3
2. CircularFifoQueue from commons-collections4 wrapped in SynchronizedQueue from Guava

It seems that there is hardly a noticable difference in performance.

▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂
Apache commons Buffer, 1000000 puts: 59 ms
▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂
Guava-wrapped Apache commons Queue, 1000000 puts: 51 ms
▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂
Apache commons Buffer, 1000000 puts with removal: 80 ms
▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂
Guava-wrapped Apache commons Queue, 1000000 puts with removal: 80 ms

Here's the repo, feel free to hack around in it: https://git.magnolia-cms.com/users/creichenbach/repos/queue-performance-benchmark/browse

Comment by Jan Haderka [ 23/Aug/16 ]

I thought that we would abandon commons for this all together and use just guava, but if it works fine, we can use also combination of both, no problem there. And since performance also seems to be comparable, IMO, just go for it.

Generated at Sun Feb 11 23:00:00 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.