Skip to content

support config for jackson buffer recycler pool#1192

Merged
pjfanning merged 4 commits intoapache:mainfrom
pjfanning:jackson-pool
Mar 15, 2024
Merged

support config for jackson buffer recycler pool#1192
pjfanning merged 4 commits intoapache:mainfrom
pjfanning:jackson-pool

Conversation

@pjfanning
Copy link
Member

  • the buffer recycler is an important performance feature in Jackson
  • Jackson 2.17 also changes the default pool implementation and this has proved an issue - see Performance regression with 2.17.x FasterXML/jackson-module-scala#672
  • my plan for Pekko is to keep the existing ThreadLocal implementation as the default even if Jackson has a different default

pool-instance = "thread-local"
# the maximum size of bounded recycler pools - must be >=1 or an IllegalArgumentException will occur
# only applies to pool-instance type "bounded"
bounded-pool-size = 100
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bounded-pool-size = 100 , is it a recommended value in prod environments ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the Jackson default - ie the default in the Jackson code

@laglangyue
Copy link
Contributor

It looks good.
Is it necessary to supplement the documentation?
https://pekko.apache.org/docs/pekko/current/serialization-jackson.html#additional-features

val streamWriteConstraints = mapper.getFactory.streamWriteConstraints()
streamWriteConstraints.getMaxNestingDepth shouldEqual maxNestingDepth
}
"support BufferRecycler (default)" in {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add an empty line.

val recyclerPool = mapper.getFactory._getRecyclerPool()
recyclerPool.getClass.getSimpleName shouldEqual "ThreadLocalPool"
}
"support BufferRecycler with config override" in {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add an empty line.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Member

@He-Pin He-Pin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks.

@pjfanning pjfanning merged commit 17577cf into apache:main Mar 15, 2024
@pjfanning pjfanning deleted the jackson-pool branch March 15, 2024 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants