Skip to content

Commit

Permalink
Improved fix for #285
Browse files Browse the repository at this point in the history 
1. Write SAME_BYTES to cacheMap when woven bytes are null
2. Fix TODO in SimpleCache::getAndInitialize, using Optional to help
   indicate cache hit for unwoven class
3. Improve test coverage (cache miss, cache hit for unwoven class)

Relates to #285.

Co-authored-by: Alexander Kriegisch <[email protected]>
Signed-off-by: KimmingLau <[email protected]>
Signed-off-by: Alexander Kriegisch <[email protected]>
  • Loading branch information
@KimmingLau @kriegaex
KimmingLau and kriegaex committed Mar 4, 2024
1 parent 1cdf711 commit 1f1d429
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 48 deletions.
7 changes: 4 additions & 3 deletions loadtime/src/main/java/org/aspectj/weaver/loadtime/Aj.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.StringTokenizer;

Expand Down Expand Up @@ -104,9 +105,9 @@ public byte[] preProcess(String className, final byte[] bytes, ClassLoader class
synchronized (classLoader) {

if (SimpleCacheFactory.isEnabled()) {
byte[] cacheBytes= laCache.getAndInitialize(className, bytes, classLoader, protectionDomain);
if (cacheBytes!=null){
return cacheBytes;
Optional<byte[]> cacheBytes = laCache.getAndInitialize(className, bytes, classLoader, protectionDomain);
if (cacheBytes != null){
return cacheBytes.orElse(null);
}
}

Expand Down
17 changes: 8 additions & 9 deletions weaver/src/main/java/org/aspectj/weaver/tools/cache/SimpleCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.zip.CRC32;

import org.aspectj.weaver.Dump;
Expand All @@ -37,7 +38,7 @@
public class SimpleCache {

private static final String SAME_BYTES_STRING = "IDEM";
private static final byte[] SAME_BYTES = SAME_BYTES_STRING.getBytes();
static final byte[] SAME_BYTES = SAME_BYTES_STRING.getBytes();

private Map<String, byte[]> cacheMap;
private boolean enabled = false;
Expand All @@ -64,22 +65,20 @@ protected SimpleCache(String folder, boolean enabled) {
}
}

public byte[] getAndInitialize(String classname, byte[] bytes,
public Optional<byte[]> getAndInitialize(String classname, byte[] bytes,
ClassLoader loader, ProtectionDomain protectionDomain) {
if (!enabled) {
return null;
}
byte[] res = get(classname, bytes);

if (Arrays.equals(SAME_BYTES, res)) {
return null;
} else {
if (res != null) {
initializeClass(classname, res, loader, protectionDomain);
}
return res;
return Optional.empty();
} else if (res != null) {
initializeClass(classname, res, loader, protectionDomain);
return Optional.of(res);
}

return null;
}

private byte[] get(String classname, byte bytes[]) {
Expand Down
95 changes: 59 additions & 36 deletions weaver/src/test/java/org/aspectj/weaver/tools/cache/SimpleClassCacheTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,60 +12,83 @@

package org.aspectj.weaver.tools.cache;

import java.io.File;

import junit.framework.TestCase;

/**
*/
public class SimpleClassCacheTest extends TestCase {
byte[] FAKE_BYTES_V1 = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
byte[] FAKE_BYTES_V2 = {1, 1, 2, 3, 4, 5, 6, 7, 8, 9};
import java.io.File;

byte[] FAKE_WOVEN_BYTES_V1 = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9,10};
byte[] FAKE_WOVEN_BYTES_V2 = {1, 1, 2, 3, 4, 5, 6, 7, 8, 9,10};
public class SimpleClassCacheTest extends TestCase {
byte[] FAKE_BYTES_V1 = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
byte[] FAKE_BYTES_V2 = { 1, 1, 2, 3, 4, 5, 6, 7, 8, 9 };

byte[] FAKE_WOVEN_BYTES_V1 = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
byte[] FAKE_WOVEN_BYTES_V2 = { 1, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };

private SimpleCache createCache() throws Exception {
return new SimpleCache(System.getProperty("java.io.tmpdir"),true);
private SimpleCache createCache() {
return new SimpleCache(System.getProperty("java.io.tmpdir"), true);
}


public void testCache() throws Exception {
public void testCache() {
String classA = "com.generated.A";
SimpleCache cache = createCache();

cache.put(classA, FAKE_BYTES_V1, FAKE_WOVEN_BYTES_V1);

// Returned woven bytes are the original ones
byte[] result = cache.getAndInitialize(classA, FAKE_BYTES_V1, null, null).orElse(null);
assertNotNull(result);
for (int i = 0; i < result.length; i++)
assertEquals(
"Cached version byte[" + i + "] should be equal to the original woven class",
result[i], FAKE_WOVEN_BYTES_V1[i]
);

// Class is properly backed up
File f = new File(System.getProperty("java.io.tmpdir") + File.separator + "com.generated.A-1164760902");
assertTrue(
"Class should be backed up with CRC 1164760902",
f.exists()
);
}

public void testDifferentVersionCache() {
String classA = "com.generated.A";
SimpleCache cache = createCache();
cache.put(classA, FAKE_BYTES_V1, FAKE_WOVEN_BYTES_V1);
cache.put(classA, FAKE_BYTES_V2, FAKE_WOVEN_BYTES_V2);

// Test the returned woven bytes are the original one
byte result[] = cache.getAndInitialize(classA, FAKE_BYTES_V1, null, null);
for(int i = 0; i < result.length; i ++){
assertEquals("Cached version byte[" +i+"] should be equal to the original woven classe",result[i],FAKE_WOVEN_BYTES_V1[i]);
}
// Returned woven bytes are the original ones for v1
byte[] result = cache.getAndInitialize(classA, FAKE_BYTES_V1, null, null).orElse(null);
assertNotNull(result);
for (int i = 0; i < result.length; i++)
assertEquals(
"Cached version v1 byte[" + i + "] should be equal to the original woven class",
result[i], FAKE_WOVEN_BYTES_V1[i]
);

// Returned woven bytes are the original ones for v2
result = cache.getAndInitialize(classA, FAKE_BYTES_V2, null, null).orElse(null);
assertNotNull(result);
for (int i = 0; i < result.length; i++)
assertEquals(
"Cached version v2 byte[" + i + "] should be equal to the original woven class",
result[i], FAKE_WOVEN_BYTES_V2[i]
);
}

// Assure the class is properly backed up in the backing folder
File f = new File (System.getProperty("java.io.tmpdir") + File.separator + "com.generated.A-1164760902");
assertTrue("Class should be backed up to backing folder, with te CRC:1164760902 ",f.exists());
public void testCacheMiss() {
String classA = "com.generated.A";
SimpleCache cache = createCache();

// Woven bytes not found in cache
assertNull(cache.getAndInitialize(classA, FAKE_BYTES_V1, null, null));
}

public void testDifferentVersionCache() throws Exception {
public void testCacheHitUnwoven() {
String classA = "com.generated.A";
SimpleCache cache = createCache();
cache.put(classA, FAKE_BYTES_V1, FAKE_WOVEN_BYTES_V1);
cache.put(classA, FAKE_BYTES_V2, FAKE_WOVEN_BYTES_V2);
cache.put(classA, FAKE_BYTES_V1, SimpleCache.SAME_BYTES);

// Test the returned woven bytes are the original one for v1
byte result[] = cache.getAndInitialize(classA, FAKE_BYTES_V1, null, null);
for(int i = 0; i < result.length; i ++){
assertEquals("Cached version v1 byte[" +i+"] should be equal to the original woven classe",result[i],FAKE_WOVEN_BYTES_V1[i]);
}

// Test the returned woven bytes are the original one for v2
result = cache.getAndInitialize(classA, FAKE_BYTES_V2, null, null);
for(int i = 0; i < result.length; i ++){
assertEquals("Cached version v2 byte[" +i+"] should be equal to the original woven classe",result[i],FAKE_WOVEN_BYTES_V2[i]);
}
// Returned woven bytes are null, indicating an unwoven class
byte[] result = cache.getAndInitialize(classA, FAKE_BYTES_V1, null, null).orElse(null);
assertNull(result);
}
}

0 comments on commit 1f1d429

Please sign in to comment.