chore(java): Reduce guava TypeToken dependency #1273
Conversation
java/fury-core/src/main/java/org/apache/fury/reflect/Primitives.java
Outdated
Show resolved
Hide resolved
| * Contains static utility methods pertaining to primitive types and their corresponding wrapper | ||
| * types. | ||
| */ | ||
| public final class Primitives { |
There was a problem hiding this comment.
We can use or extend org.apache.fury.type.TypeUtils instead
There was a problem hiding this comment.
Reused TypeUtils and removed Primitives.
| */ | ||
| public Class<? super T> getRawType() { | ||
| // For wildcard or type variable, the first bound determines the runtime type. | ||
| Class<?> rawType = getRawTypes(type).iterator().next(); |
There was a problem hiding this comment.
This may be expensive. Fury init will invoke this API many times.
There was a problem hiding this comment.
I will rewrite and update the patch.
|
|
||
| package org.apache.fury.reflect; | ||
|
|
||
| import com.google.common.reflect.TypeParameter; |
There was a problem hiding this comment.
Could we implement TypeToken by ourself without depending on any guava API. org.apache.fury.type.GenericType#build(java.lang.reflect.Type, java.util.function.Predicate<java.lang.reflect.Type>) can be taken as an example.
There was a problem hiding this comment.
Fury use TypeToken to capture generic info and for Generic Type resolve only. We don't need all features of TypeToken
There was a problem hiding this comment.
For Generic Type resolve, Fury use TypeToken to:
- Resolve covariantType/invariantType
- Create new generic Type based on other generic Types:
new TypeToken<Map<K, V>>() {}.where(new TypeParameter<K>() {}, keyType)
.where(new TypeParameter<V>() {}, valueType);There was a problem hiding this comment.
I need more time to go over the code and understand the usage properly. Can this be done in follow-up PR?
PragmaTwice
changed the title
|
Hi @nandakumar131 , are you still working on this? If not, I'd like to take over it. We're planing to make a release in next weeks. It would be great to include this change. |
|
@chaokunyang, sorry about the delay. |
Awesome! This is not urgent, you can take your time. |
nandakumar131
force-pushed
the
remove_guava_part_3
branch
from
e0ba03b to
215cb53
Compare
|
Any update? Thanks |
|
@AkiChase I'm working on first release of Fury under asf. If Nandakumar doesn't have time for this, I should be able to take over it in next two weeks. |
## What does this PR do? Removes Guava's `TypeToken` usages and replaces it with a hand-made `TypeToken` implementation. To be honest, this is mostly a copy-paste of Guava's TypeToken with removing some unnecessary stuff. This implementation can be improved and optimized. ## Related issues #1113 #1273 --------- Co-authored-by: Nandakumar Vadivelu <[email protected]>
|
Hi @nandakumar131 , are you still working on this pr? |
|
@chaokunyang, I have very limited bandwidth right now. If no one else is working on this, I can spend some time over the weekend on this. |
|
Thank you @nandakumar131 . Guava TypeToken has been removed in #1553, maybe we can close this PR now? |
|
Sure, let me close this one. |
What do these changes do?
This change reduces the dependency on guava
TypeToken. This PR doesn't completely remove the dependency, follow-up PR is required to complete the work.Related issue number
#1113