remove guava #482
remove guava #482
Conversation
| import com.netflix.servo.tag.TaggingContext; | ||
|
|
||
| import java.util.function.Function; |
There was a problem hiding this comment.
There was a problem hiding this comment.
Ah good point, I can look at fixing the breakages if they occur internally. I also took a quick look at who is still using servo and it's a bit intertwined into internal platform libraries which are also deprecated. I can also recommend full removal of servo but figured this would get some benefit in the interim.
| @@ -15,8 +15,7 @@ | |||
| */ | |||
| package com.netflix.servo.monitor; | |||
|
|
|||
| import com.google.common.base.Function; | |||
| import com.google.common.cache.Cache; | |||
| import com.github.benmanes.caffeine.cache.Cache; | |||
There was a problem hiding this comment.
If there is no usage of Monitors.newCacheMonitor internally at Netflix that would break, then I would prefer to just remove the method rather than move it over to Caffeine.
There was a problem hiding this comment.
Makes sense to me, will remove vs update to Caffeine 👍
There was a problem hiding this comment.
Looks like DynamicGauge uses it 🤔
| /** | ||
| * Intern strings used for tag keys or values. | ||
| */ | ||
| public static String intern(String v) { | ||
| return STR_CACHE.intern(v); | ||
| return v.intern(); |
There was a problem hiding this comment.
This would have very different performance implications and could cause issues. The default string table in the JDK doesn't grow and can be quite small unless customized by the user and having too many interned strings can really degrade performance.
There was a problem hiding this comment.
Agreed, I was hoping we could take the performance hit for legacy consumers or if they notice the problem we can get them to update to spectator 😆
No description provided.