Skip to content

Commit

Permalink
Polish code
Browse files Browse the repository at this point in the history
  • Loading branch information
@kylixs
kylixs committed Dec 16, 2023
1 parent e6d5f1b commit f32075d
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 37 deletions.
9 changes: 7 additions & 2 deletions ....x-3.x-plugin/src/test/java/org/apache/skywalking/apm/agent/JedisInstrumentationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import com.google.common.base.Stopwatch;
import net.bytebuddy.agent.ByteBuddyAgent;
import org.apache.skywalking.apm.agent.core.logging.core.SystemOutWriter;
import org.apache.skywalking.apm.agent.core.plugin.AbstractClassEnhancePluginDefine;
import org.apache.skywalking.apm.agent.core.plugin.ByteBuddyCoreClasses;
import org.apache.skywalking.apm.agent.core.plugin.PluginFinder;
Expand Down Expand Up @@ -56,7 +57,7 @@ public void test() throws Exception {
Assert.assertTrue(e.toString(), e.toString().contains("JedisConnectionException"));
}

System.out.println("Do re-transform class : redis.clients.jedis.Jedis ..");
log("Do re-transform class : redis.clients.jedis.Jedis ..");
Stopwatch stopwatch = Stopwatch.createStarted();

// re-transform class
Expand All @@ -65,7 +66,7 @@ public void test() throws Exception {
stopwatch.start();
instrumentation.retransformClasses(Jedis.class);
long elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS);
System.out.println("re-transform class cost: " + elapsed);
log("Re-transform class cost: " + elapsed);
}

// test after re-transform class
Expand All @@ -75,4 +76,8 @@ public void test() throws Exception {
Assert.assertTrue(e.toString(), e.toString().contains("JedisConnectionException"));
}
}

private void log(String message) {
SystemOutWriter.INSTANCE.write(message);
}
}
68 changes: 39 additions & 29 deletions ...ffer/bytebuddy-patch/src/main/java/net/bytebuddy/agent/builder/SWDescriptionStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import net.bytebuddy.pool.TypePool;
import net.bytebuddy.utility.JavaModule;
import net.bytebuddy.utility.nullability.MaybeNull;
import org.apache.skywalking.apm.agent.core.plugin.AbstractClassEnhancePluginDefine;
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;

import java.io.Serializable;
Expand All @@ -38,6 +37,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -123,15 +123,13 @@ static class SWTypeDescriptionWrapper extends TypeDescription.AbstractBase imple
*/
private static final long serialVersionUID = 1L;

private static final List<String> IGNORED_INTERFACES = Arrays.asList(EnhancedInstance.class.getName());

private static final List<String> IGNORED_FIELDS = Arrays.asList(AbstractClassEnhancePluginDefine.CONTEXT_ATTR_NAME);

/**
* Method name list cache for origin type.
* classloader hashcode -> ( typeName -> method descriptor hashcodes )
* Original type cache.
* classloader hashcode -> ( typeName -> type cache )
*/
private static Map<Integer, Map<String, List<Integer>>> classLoaderTypeMethodsCache = new ConcurrentHashMap<>();
private static final Map<Integer, Map<String, TypeCache>> CLASS_LOADER_TYPE_CACHE = new ConcurrentHashMap<>();

private static final List<String> IGNORED_INTERFACES = Arrays.asList(EnhancedInstance.class.getName());

private MethodList<MethodDescription.InDefinedShape> methods;

Expand All @@ -154,6 +152,13 @@ public SWTypeDescriptionWrapper(TypeDescription delegate, String nameTrait, Clas
this.typeName = typeName;
}

private TypeCache getTypeCache() {
int classLoaderHashCode = classLoader != null ? classLoader.hashCode() : 0;
Map<String, TypeCache> typeCacheMap = CLASS_LOADER_TYPE_CACHE.computeIfAbsent(classLoaderHashCode, k -> new ConcurrentHashMap<>());
TypeCache typeCache = typeCacheMap.computeIfAbsent(typeName, k -> new TypeCache(typeName));
return typeCache;
}

@Override
public TypeList.Generic getInterfaces() {
if (this.interfaces == null) {
Expand All @@ -175,14 +180,16 @@ public TypeList.Generic getInterfaces() {
public FieldList<FieldDescription.InDefinedShape> getDeclaredFields() {
if (this.fields == null) {
FieldList<FieldDescription.InDefinedShape> declaredFields = delegate.getDeclaredFields();
if (declaredFields.stream()
.anyMatch(f -> f.getName().contains(nameTrait) || IGNORED_FIELDS.contains(f.getName()))) {
// Remove dynamic field tokens generated by SkyWalking
TypeCache typeCache = getTypeCache();
if (typeCache.fieldNames == null) {
// save origin fields
typeCache.fieldNames = declaredFields.stream().map(WithRuntimeName::getName).collect(Collectors.toSet());
fields = declaredFields;
} else {
// return origin fields
fields = new FieldList.Explicit<>(declaredFields.stream()
.filter(f -> !f.getName().contains(nameTrait) && !IGNORED_FIELDS.contains(f.getName()))
.filter(f -> typeCache.fieldNames.contains(f.getName()))
.collect(Collectors.toList()));
} else {
fields = declaredFields;
}
}
return fields;
Expand All @@ -192,24 +199,17 @@ public FieldList<FieldDescription.InDefinedShape> getDeclaredFields() {
public MethodList<MethodDescription.InDefinedShape> getDeclaredMethods() {
if (this.methods == null) {
MethodList<MethodDescription.InDefinedShape> declaredMethods = delegate.getDeclaredMethods();

// get cache for classloader
int classLoaderHashCode = classLoader != null ? classLoader.hashCode() : 0;
Map<String, List<Integer>> typeMethodsCache = classLoaderTypeMethodsCache.computeIfAbsent(classLoaderHashCode, k -> new ConcurrentHashMap<>());

// get cache for type
List<Integer> originMethods = typeMethodsCache.get(typeName);
if (originMethods == null) {
TypeCache typeCache = getTypeCache();
if (typeCache.methodCodes == null) {
// save original methods
originMethods = declaredMethods.stream().map(m -> m.toString().hashCode()).collect(Collectors.toList());
typeMethodsCache.put(typeName, originMethods);
typeCache.methodCodes = declaredMethods.stream().map(m -> m.toString().hashCode()).collect(Collectors.toSet());
methods = declaredMethods;
} else {
// Return original methods in the same order, remove dynamic method tokens generated by SkyWalking
// Removing generated methods for delegating superclass calls, such as Jedis.
Map<Integer, MethodDescription.InDefinedShape> methodMap = declaredMethods.stream()
.collect(Collectors.toMap(m -> m.toString().hashCode(), v -> v, (k1, k2) -> k2));
methods = new MethodList.Explicit<>(originMethods.stream().map(methodMap::get).collect(Collectors.toList()));
// return original methods in the same order, remove dynamic method tokens generated by SkyWalking and ByteBuddy
// remove generated methods for delegating superclass methods, such as Jedis.
methods = new MethodList.Explicit<>(declaredMethods.stream()
.filter(m -> typeCache.methodCodes.contains(m.toString().hashCode()))
.collect(Collectors.toList()));
}
}
return methods;
Expand Down Expand Up @@ -353,4 +353,14 @@ public int getModifiers() {
return delegate.getModifiers();
}
}

static class TypeCache {
private String typeName;
private Set<Integer> methodCodes;
private Set<String> fieldNames;

public TypeCache(String typeName) {
this.typeName = typeName;
}
}
}
26 changes: 20 additions & 6 deletions ...rc/main/java/org/apache/skywalking/apm/agent/bytebuddy/SWMethodGraphCompilerDelegate.java
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,21 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

package org.apache.skywalking.apm.agent.bytebuddy;

import net.bytebuddy.description.method.MethodDescription;
Expand Down Expand Up @@ -63,16 +81,12 @@ public Node locate(MethodDescription.SignatureToken token) {

@Override
public NodeList listNodes() {
// sort nodes (methods) to generate same cache field order when retransform class
// sort nodes (methods) to generate same cache field order when re-transform class
NodeList nodeList = origin.listNodes();
List<Node> nodes = nodeList.stream().map(n -> new Pair<Integer, Node>(n.getRepresentative().toString().hashCode(), n))
List<Node> nodes = nodeList.stream().map(n -> new Pair<Integer, Node>(n.getRepresentative().hashCode(), n))
.sorted(Comparator.comparing(p -> p.first))
.map(p -> p.second)
.collect(Collectors.toList());

// List<Node> nodes = nodeList.stream()
// .sorted(Comparator.comparing(n -> n.getRepresentative().hashCode()))
// .collect(Collectors.toList());
return new NodeList(nodes);
}
}
Expand Down

0 comments on commit f32075d

Please sign in to comment.