-
Notifications
You must be signed in to change notification settings - Fork 134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: avoid the double evaluation of entityId in ClusterSharding #1304
Conversation
cluster-sharding/src/main/scala/org/apache/pekko/cluster/sharding/ClusterSharding.scala
Outdated
Show resolved
Hide resolved
...ed/src/main/scala/org/apache/pekko/cluster/sharding/typed/internal/ClusterShardingImpl.scala
Outdated
Show resolved
Hide resolved
cluster-sharding/src/main/scala/org/apache/pekko/cluster/sharding/ClusterSharding.scala
Outdated
Show resolved
Hide resolved
Seems like duplicate code, can it be abstracted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blocking - needs discussion on the dev list because of the ongoing release discussion
Use the cacheable partial function, and then verify it works. var calTime: Int = 0
def entityId(value: Any): String = {
calTime = calTime + 1
value match {
case "wrong" => null
case _ => String.valueOf(value)
}
}
def extractEntityIdFromExtractor: PartialFunction[Any, Any] =
new scala.runtime.AbstractPartialFunction[Any, (String, Any)] {
var cache: String = _
override def isDefinedAt(msg: Any): Boolean = {
cache = entityId(msg)
cache != null
}
override def apply(x: Any): (String, Any) = (cache, x)
}
lazy val xx = extractEntityIdFromExtractor
lazy val yy: PartialFunction[Any, Any] = {
case message if entityId(message) != null => (entityId(message), message)
}
assert(!xx.isDefinedAt("wrong"))
assert(!yy.isDefinedAt("wrong"))
calTime = 0
val health = "health"
println(xx.isDefinedAt(health))
println(xx(health))
println(s"caltime = $calTime")
println("=====")
calTime = 0
println(yy.isDefinedAt(health))
println(yy(health))
println(s"caltime = $calTime")
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@Roiocam is it ok to get this merged now? |
I have no objections to this, but it would be better if there were reviews from others. |
...ed/src/main/scala/org/apache/pekko/cluster/sharding/typed/internal/ClusterShardingImpl.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Found something we could optimized it.