You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "pjfanning (via GitHub)" <gi...@apache.org> on 2024/03/14 19:27:38 UTC
[PR] support config for jackson buffer recycler pool [incubator-pekko]
pjfanning opened a new pull request, #1192:
URL: https://github.com/apache/incubator-pekko/pull/1192
* 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 https://github.com/FasterXML/jackson-module-scala/issues/672
* my plan for Pekko is to keep the existing ThreadLocal implementation as the default even if Jackson has a different default
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] support config for jackson buffer recycler pool [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #1192:
URL: https://github.com/apache/incubator-pekko/pull/1192#discussion_r1525889946
##########
serialization-jackson/src/main/resources/reference.conf:
##########
@@ -35,6 +35,17 @@ pekko.serialization.jackson {
migrations {
}
+ # Controls the Buffer Recycler Pool implementation used by Jackson.
+ # https://javadoc.io/static/com.fasterxml.jackson.core/jackson-core/2.16.2/com/fasterxml/jackson/core/util/JsonRecyclerPools.html
+ # The default is "thread-local" which is the same as the default in Jackson 2.16.
+ buffer-recycler {
+ # the supported values are "thread-local", "lock-free", "shared-lock-free", "concurrent-deque",
+ # "shared-concurrent-deque", "bounded"
+ pool-instance = "thread-local"
Review Comment:
could you read https://github.com/FasterXML/jackson-module-scala/issues/672 ?
until Jackson has a sensible default, we will need to stick with the thread-local
I am a committer on the Jackson project. I know quite a bit about Jackson code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] support config for jackson buffer recycler pool [incubator-pekko]
Posted by "laglangyue (via GitHub)" <gi...@apache.org>.
laglangyue commented on PR #1192:
URL: https://github.com/apache/incubator-pekko/pull/1192#issuecomment-1998826886
It looks good.
Is it necessary to supplement the documentation?
https://pekko.apache.org/docs/pekko/current/serialization-jackson.html#additional-features
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] support config for jackson buffer recycler pool [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning merged PR #1192:
URL: https://github.com/apache/incubator-pekko/pull/1192
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] support config for jackson buffer recycler pool [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #1192:
URL: https://github.com/apache/incubator-pekko/pull/1192#discussion_r1525889946
##########
serialization-jackson/src/main/resources/reference.conf:
##########
@@ -35,6 +35,17 @@ pekko.serialization.jackson {
migrations {
}
+ # Controls the Buffer Recycler Pool implementation used by Jackson.
+ # https://javadoc.io/static/com.fasterxml.jackson.core/jackson-core/2.16.2/com/fasterxml/jackson/core/util/JsonRecyclerPools.html
+ # The default is "thread-local" which is the same as the default in Jackson 2.16.
+ buffer-recycler {
+ # the supported values are "thread-local", "lock-free", "shared-lock-free", "concurrent-deque",
+ # "shared-concurrent-deque", "bounded"
+ pool-instance = "thread-local"
Review Comment:
could you read https://github.com/FasterXML/jackson-module-scala/issues/672 ?
until Jackson has a sensible default, we will need to stick with the thread-local
I am a committer on the Jackson project. I know quite a bit about Jackson code.
Virtual Thread users would probably be best off using the `sharedLockFree` pool. But those users can override the config by using `application.conf`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] support config for jackson buffer recycler pool [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #1192:
URL: https://github.com/apache/incubator-pekko/pull/1192#discussion_r1526269034
##########
serialization-jackson/src/test/scala/org/apache/pekko/serialization/jackson/JacksonFactorySpec.scala:
##########
@@ -72,5 +73,29 @@ class JacksonFactorySpec extends TestKit(ActorSystem("JacksonFactorySpec"))
val streamWriteConstraints = mapper.getFactory.streamWriteConstraints()
streamWriteConstraints.getMaxNestingDepth shouldEqual maxNestingDepth
}
+ "support BufferRecycler (default)" in {
Review Comment:
add an empty line.
##########
serialization-jackson/src/test/scala/org/apache/pekko/serialization/jackson/JacksonFactorySpec.scala:
##########
@@ -72,5 +73,29 @@ class JacksonFactorySpec extends TestKit(ActorSystem("JacksonFactorySpec"))
val streamWriteConstraints = mapper.getFactory.streamWriteConstraints()
streamWriteConstraints.getMaxNestingDepth shouldEqual maxNestingDepth
}
+ "support BufferRecycler (default)" in {
+ val bindingName = "testJackson"
+ val jacksonConfig = JacksonObjectMapperProvider.configForBinding(bindingName, defaultConfig)
+ val mapper = JacksonObjectMapperProvider.createObjectMapper(
+ bindingName, None, objectMapperFactory, jacksonConfig, dynamicAccess, None)
+ val recyclerPool = mapper.getFactory._getRecyclerPool()
+ recyclerPool.getClass.getSimpleName shouldEqual "ThreadLocalPool"
+ }
+ "support BufferRecycler with config override" in {
Review Comment:
add an empty line.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] support config for jackson buffer recycler pool [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #1192:
URL: https://github.com/apache/incubator-pekko/pull/1192#discussion_r1525686612
##########
serialization-jackson/src/main/resources/reference.conf:
##########
@@ -35,6 +35,17 @@ pekko.serialization.jackson {
migrations {
}
+ # Controls the Buffer Recycler Pool implementation used by Jackson.
+ # https://javadoc.io/static/com.fasterxml.jackson.core/jackson-core/2.16.2/com/fasterxml/jackson/core/util/JsonRecyclerPools.html
+ # The default is "thread-local" which is the same as the default in Jackson 2.16.
+ buffer-recycler {
+ # the supported values are "thread-local", "lock-free", "shared-lock-free", "concurrent-deque",
+ # "shared-concurrent-deque", "bounded"
+ pool-instance = "thread-local"
Review Comment:
it's a thread-local based? In https://github.com/eclipse-vertx/vert.x/pull/5084/ vert.x guy sent a VT friendly about this too, should we have that in mind?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] support config for jackson buffer recycler pool [incubator-pekko]
Posted by "laglangyue (via GitHub)" <gi...@apache.org>.
laglangyue commented on code in PR #1192:
URL: https://github.com/apache/incubator-pekko/pull/1192#discussion_r1525674492
##########
serialization-jackson/src/main/resources/reference.conf:
##########
@@ -35,6 +35,17 @@ pekko.serialization.jackson {
migrations {
}
+ # Controls the Buffer Recycler Pool implementation used by Jackson.
+ # https://javadoc.io/static/com.fasterxml.jackson.core/jackson-core/2.16.2/com/fasterxml/jackson/core/util/JsonRecyclerPools.html
+ # The default is "thread-local" which is the same as the default in Jackson 2.16.
+ buffer-recycler {
+ # the supported values are "thread-local", "lock-free", "shared-lock-free", "concurrent-deque",
+ # "shared-concurrent-deque", "bounded"
+ 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
Review Comment:
` bounded-pool-size = 100` , is it a recommended value in prod environments ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] support config for jackson buffer recycler pool [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #1192:
URL: https://github.com/apache/incubator-pekko/pull/1192#discussion_r1525889946
##########
serialization-jackson/src/main/resources/reference.conf:
##########
@@ -35,6 +35,17 @@ pekko.serialization.jackson {
migrations {
}
+ # Controls the Buffer Recycler Pool implementation used by Jackson.
+ # https://javadoc.io/static/com.fasterxml.jackson.core/jackson-core/2.16.2/com/fasterxml/jackson/core/util/JsonRecyclerPools.html
+ # The default is "thread-local" which is the same as the default in Jackson 2.16.
+ buffer-recycler {
+ # the supported values are "thread-local", "lock-free", "shared-lock-free", "concurrent-deque",
+ # "shared-concurrent-deque", "bounded"
+ pool-instance = "thread-local"
Review Comment:
could you read https://github.com/FasterXML/jackson-module-scala/issues/672 ?
until Jackson has a sensible default, we will need to stick with the thread-local
I am a committer on the Jackson project. I know quite a bit about Jackson code.
Virtual Thread users would probably be best off using the `shared-lock-free` pool. But those users can override the config by using `application.conf`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] support config for jackson buffer recycler pool [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #1192:
URL: https://github.com/apache/incubator-pekko/pull/1192#discussion_r1526852938
##########
serialization-jackson/src/test/scala/org/apache/pekko/serialization/jackson/JacksonFactorySpec.scala:
##########
@@ -72,5 +73,29 @@ class JacksonFactorySpec extends TestKit(ActorSystem("JacksonFactorySpec"))
val streamWriteConstraints = mapper.getFactory.streamWriteConstraints()
streamWriteConstraints.getMaxNestingDepth shouldEqual maxNestingDepth
}
+ "support BufferRecycler (default)" in {
+ val bindingName = "testJackson"
+ val jacksonConfig = JacksonObjectMapperProvider.configForBinding(bindingName, defaultConfig)
+ val mapper = JacksonObjectMapperProvider.createObjectMapper(
+ bindingName, None, objectMapperFactory, jacksonConfig, dynamicAccess, None)
+ val recyclerPool = mapper.getFactory._getRecyclerPool()
+ recyclerPool.getClass.getSimpleName shouldEqual "ThreadLocalPool"
+ }
+ "support BufferRecycler with config override" in {
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] support config for jackson buffer recycler pool [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #1192:
URL: https://github.com/apache/incubator-pekko/pull/1192#discussion_r1525890660
##########
serialization-jackson/src/main/resources/reference.conf:
##########
@@ -35,6 +35,17 @@ pekko.serialization.jackson {
migrations {
}
+ # Controls the Buffer Recycler Pool implementation used by Jackson.
+ # https://javadoc.io/static/com.fasterxml.jackson.core/jackson-core/2.16.2/com/fasterxml/jackson/core/util/JsonRecyclerPools.html
+ # The default is "thread-local" which is the same as the default in Jackson 2.16.
+ buffer-recycler {
+ # the supported values are "thread-local", "lock-free", "shared-lock-free", "concurrent-deque",
+ # "shared-concurrent-deque", "bounded"
+ 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
Review Comment:
this is the Jackson default - ie the default in the Jackson code
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] support config for jackson buffer recycler pool [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #1192:
URL: https://github.com/apache/incubator-pekko/pull/1192#discussion_r1526265213
##########
serialization-jackson/src/main/resources/reference.conf:
##########
@@ -35,6 +35,17 @@ pekko.serialization.jackson {
migrations {
}
+ # Controls the Buffer Recycler Pool implementation used by Jackson.
+ # https://javadoc.io/static/com.fasterxml.jackson.core/jackson-core/2.16.2/com/fasterxml/jackson/core/util/JsonRecyclerPools.html
+ # The default is "thread-local" which is the same as the default in Jackson 2.16.
+ buffer-recycler {
+ # the supported values are "thread-local", "lock-free", "shared-lock-free", "concurrent-deque",
+ # "shared-concurrent-deque", "bounded"
+ pool-instance = "thread-local"
Review Comment:
Thanks, just got time to look into this in detail , would you mind mention this in the comments above?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org