Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

kilim.S_L.class not generated #17

Closed
tstockwell opened this Issue · 3 comments

2 participants

@tstockwell

Here is a test case that contains a Pausable method.
When fed into the kilim weaver, kilim should not only weave the Pausable method but it should also generate a class file named kilim.S_L (which is used to manage the methods state info, in this case a long). However, the weaver fails to generate kilim.S_L.

import junit.framework.TestCase;
import kilim.Pausable;
import kilim.Task;

public class KilimTests extends TestCase {

  public void testSleep() throws Throwable {
    runTest(new Task() {
      public void execute() throws Pausable, Exception {
        long start= System.currentTimeMillis();
        Task.sleep(1000); 
        long time= System.currentTimeMillis() - start;
        assertTrue("Task.sleep didnt sleep long enough, only slept for "+time+" milliseconds", start+1000 <= System.currentTimeMillis());
      }
    });
  }

}
@tstockwell

OK, I figured this one out, it is specific to the KilimBuilder Eclipse plugin. It does not happen in the commandline kilim weaver. However, Kilim requires a change in order to work with KilimBuilder. This issue is this...
The kilim.analysis.ClassWeaver class maintains a static list of state classes that have been generated. Therefore, when used in Eclipse, if kilim.S_L is generated once when one eclipse project is built, then it will not be generated again in a second eclipse project that requires it.

I propose a change to the ClassWeaver.createStateClass method.
Instead of caching the names of state classes, cache the ClassInfo created for the state class. Then every time a class needs a state class, the appropriate ClassInfo is fetched from the cache and added to the ClassWeaver (making sure that a particular state class is never added to a ClassWeaver more than once).

There are already checks in kilim.tools.RuntimeWeaver and kilim.tools.Weaver that prevent the same state class from being written more than once, so this change should not cause any significant decrease in performance.

I will post a diff for this change...

@tstockwell

Here is a diff for the changes I made. I have tested this change, seems to work fine...


21a21
> import java.util.HashMap;
44c45
<     static HashSet<String> stateClasses = new HashSet<String>();
---
>     static HashMap<String, ClassInfo> stateClasses = new HashMap<String, ClassInfo>();
200,222c201,228
<         if (stateClasses.contains(className)) {
<             return className;
<         }
<         stateClasses.add(className);
<         ClassWriter cw = new ClassWriter(false);
<         cw.visit(V1_1, ACC_PUBLIC | ACC_FINAL, className, null, "kilim/State", null);
< 
<         // Create default constructor
<         // <init>() {
<         // super(); // call java/lang/Object.<init>()
<         // }
<         MethodVisitor mw = cw.visitMethod(ACC_PUBLIC, "<init>", "()V", null, null);
<         mw.visitInsn(ALOAD_0);
<         mw.visitMethodInsn(INVOKESPECIAL, STATE_CLASS, "<init>", "()V");
<         mw.visitInsn(RETURN);
<         // this code uses a maximum of one stack element and one local variable
<         mw.visitMaxs(1, 1);
<         mw.visitEnd();
<         // create fields of the appropriate type.
<         for (ValInfo vi : valInfoList) {
<             cw.visitField(ACC_PUBLIC, vi.fieldName, vi.fieldDesc(), null, null);
<         }
<         addClassInfo(new ClassInfo(className, cw.toByteArray()));
---
>         ClassInfo classInfo= null;
>         synchronized (stateClasses) {
>             classInfo= stateClasses.get(className);
>             if (classInfo == null) {
>                 ClassWriter cw = new ClassWriter(false);
>                 cw.visit(V1_1, ACC_PUBLIC | ACC_FINAL, className, null, "kilim/State", null);
> 
>                 // Create default constructor
>                 // <init>() {
>                 // super(); // call java/lang/Object.<init>()
>                 // }
>                 MethodVisitor mw = cw.visitMethod(ACC_PUBLIC, "<init>", "()V", null, null);
>                 mw.visitInsn(ALOAD_0);
>                 mw.visitMethodInsn(INVOKESPECIAL, STATE_CLASS, "<init>", "()V");
>                 mw.visitInsn(RETURN);
>                 // this code uses a maximum of one stack element and one local variable
>                 mw.visitMaxs(1, 1);
>                 mw.visitEnd();
>                 // create fields of the appropriate type.
>                 for (ValInfo vi : valInfoList) {
>                     cw.visitField(ACC_PUBLIC, vi.fieldName, vi.fieldDesc(), null, null);
>                 }
>                 classInfo= new ClassInfo(className, cw.toByteArray());
>                 stateClasses.put(className, classInfo);
>             }
>     }
>         if (!classInfoList.contains(classInfo))
>           addClassInfo(classInfo);

@kilim kilim referenced this issue from a commit
@sriram-srinivasan sriram-srinivasan Fix for Issue #17 thanks to ted stockwell.
From the issue page:
"The kilim.analysis.ClassWeaver class maintains a static list of state classes that have been generated. Therefore, when used in Eclipse, if kilim.S_L is generated once when one eclipse project is built, then it will not be generated again in a second eclipse project that requires it.
"

	modified:   src/kilim/analysis/ClassWeaver.java
d29e9f9
@kilim
Owner

Thanks much for the diagnosis and the fix.

Fixed in commit d29e9f9

@kilim kilim closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.