-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Optimize Plugin #3166
base: master
Are you sure you want to change the base?
Optimize Plugin #3166
Conversation
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.
review and suggested improvement for your code:
Review:
- Thread Safety: In the latest code, you've introduced a ConcurrentHashMap (methodMap) to store method interception status, which is a good approach for concurrent access.
- Efficiency: Using ConcurrentHashMap for methodMap ensures efficient thread-safe computation and storage of intercepted methods.
- Code Clarity: The use of MapUtil.computeIfAbsent is concise and clear, improving readability compared to manually checking and inserting into methodMap.
Improvement Suggestions: - Initialization of methodMap: Ensure that methodMap is properly initialized in the constructor of Plugin class. You've declared it, but ensure it's initialized correctly.
java
Copy code
private final ConcurrentMap<Method, Boolean> methodMap;
private Plugin(Object target, Interceptor interceptor, Map<Class<?>, Set> signatureMap) {
this.target = target;
this.interceptor = interceptor;
this.signatureMap = signatureMap;
this.methodMap = new ConcurrentHashMap<>();
}
2. Consistency: Ensure that interceptor field is properly used in all code paths where interception occurs. It seems correctly handled already.
3. Error Handling: Consider adding appropriate error handling or logging within the invoke method to handle any potential exceptions thrown during method invocation or interception.
Code Consolidation:
You can consolidate the intercepted logic with the method invocation for improved readability:
java
Copy code
@OverRide
public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
boolean intercepted = MapUtil.computeIfAbsent(methodMap, method, key -> {
Set methods = signatureMap.get(method.getDeclaringClass());
return methods != null && methods.contains(method);
});
if (intercepted) {
return interceptor.intercept(new Invocation(target, method, args));
} else {
return method.invoke(target, args);
}
}
This ensures that the interception logic is clear and integrated with method invocation in a straightforward manner.
Summary:
Your latest code shows improvement in thread safety and efficiency with the use of ConcurrentHashMap. Ensure proper initialization and error handling for robustness. The consolidation of interception logic and method invocation can further enhance code readability.
modified. |
The method object in the invoke method parameters != The method object saved in the signatureMap. When calling methods.contains(method), method Methods#equals() will definitely be called, and this method has low performance. Adding a Map to cache the results can avoid calling this method.