Hi Alexander, Yes, I agree. Let's close this. Thanks for all your effort.
regards, Yongle On Thu, May 31, 2018 at 9:09 PM, Alexander Kriegisch <alexan...@kriegisch.name> wrote: > Hi Yongle, > > in the log you can differentiate them, indeed. I knew that. The > misunderstanding was that somehow you wanted to be able to use the internal > map in the aspect to uniquely identify each thread, i.e. I thought despite > the fact that you keep the creator thread ID in the thread instance itself > via ITD, you want globally unique thread IDs (that's what ID means to me and > that's how it works in the JVM). So your choice of naming was a little > ambiguous. My solution provides unique thread IDs, so the creator ID is only > sugar on the cake and not strictly needed to identify a thread. Your > solution does depend on the creator ID, but as that is what you want and it > is good enough for you, of course you can use your own approach too. My > solution also has the advantage that by checking out the map you can easily > determine how many threads there are in total per class or across the JVM > without any overhead, which is what I use in my test program to calculate > the stats. If you do not need that or are fine with scanning log files if > you need to know, okay. As I said, my analysis showed no noticeable > difference in performance due to locking, so IMO it is not really more > expensive than yours. > > Let us close this thread. For me it was a nice finger exercise because apart > from the aspects it has been a while that I thought about multi-threading in > this context. > > Regards > -- > Alexander Kriegisch > https://scrum-master.de > > > > Yongle Zhang schrieb am 01.06.2018 05:50: > > Hi Alexander, > > Thanks for your time and effort. > > In fact my aspect is enough for my use case. I think I didn’t clearly > explain my scenario. Here is some log printed on a simple thrift server with > my aspect: > > [Created Runnable] PID=27961@localhost, TID=1, > org.apache.thrift.server.TThreadPoolServer$WorkerProcess(instanceID=0, > creatorTID=1) > [Submit Runnable] Executor PID=27961@localhost, TID=1, > org.apache.thrift.server.TThreadPoolServer$WorkerProcess(instanceID=0, > creatorTID=1) > [Running Runnable/Callable] PID=27961@localhost, TID=10, > org.apache.thrift.server.TThreadPoolServer$WorkerProcess(instanceID=0, > creatorTID=1) > [Finished Runnable/Callable] PID=27961@localhost, TID=10, > org.apache.thrift.server.TThreadPoolServer$WorkerProcess(instanceID=0, > creatorTID=1) > > What I want to know: > > Thread TID=1 submitted a Runnable (instanceID=0, creatorTID=1) to an > Executor. > Thread TID=10 (which is some thread in the thread pool used by that > Executor) started running that Runnable (instanceID=0, creatorTID=1). > > And the problem you pointed out in your example - 2 Runnables on line 1 and > line 5 has the same instanceID: > > [Create Runnable/Callable] TID=1, > de.scrum_master.app.SomeRunnable(instanceID=0, creatorTID=1) > [Create Runnable/Callable] TID=1, > de.scrum_master.app.Application$InnerStaticRunnable(instanceID=0, > creatorTID=1) > [Create Runnable/Callable] TID=1, > de.scrum_master.app.Application$InnerRunnable(instanceID=0, creatorTID=1) > [Create Runnable/Callable] TID=1, > de.scrum_master.app.SomeRunnable(instanceID=1, creatorTID=1) > [Create Runnable/Callable] TID=12, > de.scrum_master.app.SomeRunnable(instanceID=0, creatorTID=12) > [Create Runnable/Callable] TID=1, > de.scrum_master.app.RunnableBase(instanceID=0, creatorTID=1) > Duration (minus sleeping time) = 22 ms > Unexpected counter value for class de.scrum_master.app.SomeRunnable: > expected=3, actual=2 > Unexpected number of total IDs: expected=6, actual=5 > > This problem doesn’t matter because they have different creatorTIDs > (SomeRunnable(instanceID=0, creatorTID=1) vs. SomeRunnable(instanceID=0, > creatorTID=12)), so they can be differentiated from each other. > > I understand that ThreadLocal doesn’t synchronize or lock anything, and > that’s the reason I used it - to remove synchronization as much as possible. > And it’s OK that some Runnables have the same instanceID, because they will > definitely have different creator Thread IDs. Please feel free to tell me > where I didn’t explain clearly and lost you. > > Your fix for “extra synchronization needed in your method” is elegant! I’ll > definitely try both solutions. > > Again thanks for your time into this. I learnt a lot. Really appreciate it. > > regards, Yongle > > > > > > > On May 30, 2018 at 9:24:28 AM, Alexander Kriegisch > (alexan...@kriegisch.name) wrote: > > @Yongle: Andy already fixed the original problem. I retested the developer > snapshot, see my report at > https://bugs.eclipse.org/bugs/show_bug.cgi?id=535086. so if you like to give > your original code another spin, feel free to test by yourself. > > @Andy: Thanks! > > -- > Alexander Kriegisch > https://scrum-master.de > > > Andy Clement schrieb am 26.05.2018 02:31: > > @Alexander > > I wondered if you had an opinion on this. I’ve been working on this under > https://bugs.eclipse.org/bugs/show_bug.cgi?id=535086 . I can’t decide if you > should have to specify privileged in order for pertypewithin to match > against types not visible from the aspect. Currently I have it so that > pertypewithin will no longer match private types or default vis types in > another package unless you specify privileged. If you do specify privilege > the visibility is raised for those types during weaving so they are > accessible (the auto raising of visibility is not unusual, we do it for > other reasons at times). > > > > > On May 24, 2018, at 10:14 AM, Andy Clement <andrew.clem...@gmail.com> wrote: > > I think there is a visibility issue here - the creation of the aspect fails > because in trying to create it we use reflection to invoke a method in the > affected type (the type pertypewithin is hitting) and that reflection is > failing (silently) because the type isn't accessible. The type visibility > should probably have been raised if the type is hit by pertypewithin. > > > On 19 May 2018 at 04:01, Alexander Kriegisch <alexan...@kriegisch.name> > wrote: >> >> Okay, I think I figured it out. It is not per se a problem with the 3rd >> party code, I can reproduce it with or without Thrift. The core problem is >> that the inner class you want to instrument in the library is non-public. To >> be exact, it is a private, non-static inner class. But what really matters >> is that it is anything but public/protected, static or not is not so >> important. >> >> To Andy Clement: Maybe this is a shortcoming in AspectJ and we need a >> Bugzilla ticket for it, but first I am going to post some sample code here: >> >> ________________________________ >> >> package de.scrum_master.app; >> >> public class Application { >> private static class InnerStaticRunnable implements Runnable { >> @Override >> public void run() { >> System.out.println("Running inner static runnable"); >> } >> } >> >> private class InnerRunnable implements Runnable { >> @Override >> public void run() { >> System.out.println("Running inner runnable"); >> } >> } >> >> public void doSomething() { >> new InnerRunnable().run(); >> } >> >> public static void main(String[] args) { >> new SomeRunnable().run(); >> new Application.InnerStaticRunnable().run(); >> new Application().doSomething(); >> } >> } >> >> ________________________________ >> >> package de.scrum_master.aspect; >> >> privileged aspect MyRunnables { >> public interface InstrumentedRunnable {} >> private long InstrumentedRunnable.myid = -1; >> public long InstrumentedRunnable.getMyid() { return myid; } >> public void InstrumentedRunnable.setMyid(long id) { myid = id; } >> >> declare parents : Runnable+ implements InstrumentedRunnable; >> >> after() returning(InstrumentedRunnable r) : >> call(java.lang.Runnable+.new(..)) { >> System.out.println(thisJoinPoint); >> System.out.println(" Runnable: " + r); >> System.out.println(" Has aspect: " + >> PerRunnable.hasAspect(r.getClass())); >> long id = PerRunnable.aspectOf(r.getClass()).getCounter(); >> r.setMyid(id); >> PerRunnable.aspectOf(r.getClass()).incrementCounter(); >> } >> } >> >> ________________________________ >> >> package de.scrum_master.aspect; >> >> privileged aspect PerRunnable pertypewithin(java.lang.Runnable+) { >> public long counter = 0; >> public long getCounter() { return counter; } >> public void incrementCounter() { counter++; } >> >> after() : staticinitialization(*) { >> System.out.println("getWithinTypeName() = " + getWithinTypeName()); >> } >> } >> >> ________________________________ >> >> Now let's run the code after Ajc compilation and check the console log: >> >> getWithinTypeName() = de.scrum_master.app.SomeRunnable >> call(de.scrum_master.app.SomeRunnable()) >> Runnable: de.scrum_master.app.SomeRunnable@5674cd4d >> Has aspect: true >> Running some runnable >> getWithinTypeName() = de.scrum_master.app.Application$InnerStaticRunnable >> >> call(de.scrum_master.app.Application.InnerStaticRunnable(Application.InnerStaticRunnable)) >> Runnable: de.scrum_master.app.Application$InnerStaticRunnable@65b54208 >> Has aspect: false >> Exception in thread "main" org.aspectj.lang.NoAspectBoundException >> at de.scrum_master.aspect.PerRunnable.aspectOf(PerRunnable.aj:1) >> at >> de.scrum_master.aspect.MyRunnables.ajc$afterReturning$de_scrum_master_aspect_MyRunnables$1$8a935d86(MyRunnables.aj:15) >> at de.scrum_master.app.Application.main(Application.java:24) >> >> ________________________________ >> >> Please note "Has aspect: false" right before the exception. Now change the >> inner classes to public or protected and the code works: >> >> getWithinTypeName() = de.scrum_master.app.SomeRunnable >> call(de.scrum_master.app.SomeRunnable()) >> Runnable: de.scrum_master.app.SomeRunnable@5674cd4d >> Has aspect: true >> Running some runnable >> getWithinTypeName() = de.scrum_master.app.Application$InnerStaticRunnable >> call(de.scrum_master.app.Application.InnerStaticRunnable()) >> Runnable: de.scrum_master.app.Application$InnerStaticRunnable@65b54208 >> Has aspect: true >> Running inner static runnable >> getWithinTypeName() = de.scrum_master.app.Application$InnerRunnable >> call(de.scrum_master.app.Application.InnerRunnable(Application)) >> Runnable: de.scrum_master.app.Application$InnerRunnable@6b884d57 >> Has aspect: true >> Running inner runnable >> >> ________________________________ >> >> Any comments, Andy? >> >> -- >> >> Alexander Kriegisch >> https://scrum-master.de >> >> >> >> Yongle Zhang schrieb am 18.05.2018 04:03: >> >> More information >> >> The part of the code that’s causing the exception - this is decompiled >> from the woven .class file: >> >> public class TThreadPoolServer { >> >> public void serve() { >> >> . . . >> >> TThreadPoolServer.WorkerProcess var13; >> >> TThreadPoolServer.WorkerProcess var10000 = var13 = new >> >> TThreadPoolServer.WorkerProcess(client, (<undefinedtype>)var10); >> >> // The exception says here afterReturning throws the exception >> >> >> >> EpredRunnablesCallables.aspectOf().ajc$afterReturning$EpredRunnablesCallables$1$8a935d86(var13); >> >> . . . >> >> } >> >> // inner class >> private class WorkerProcess implements Runnable, >> InstrumentedRunnableCallable { >> public long myid; // inserted by aspectj >> >> . . . >> >> } >> >> } >> >> Question: how does pertypewithin() work? what’s its scope? >> >> For example, pertypewithin(Runnable+) - does it work every class in the >> classpath, even including those in rt.jar? When does it create instance for >> every class that implements Runnable (after class loading, on demand, …)? >> >> Thank you! >> >> >> >> >> On May 17, 2018 at 4:36:23 PM, Yongle Zhang (yng...@gmail.com) wrote: >> >> Hi, >> >> Problem >> >> I have some aspects trying to insert an ID for every class that implements >> Runnable. >> >> My aspects (provided below) works fine for a simple test in which 1) I >> wrote my own MyRunnable class implementing Runnable, 2) I have a simple main >> function that creates and runs a thread using MyRunnable. >> >> However, when I use it to instrument apache thrift library, it gives me >> org.aspectj.lang.NoAspectBoundException exception. >> >> I use compile-time weaving. The compile-time weaving finishes >> successfully, and the instrumented .class code shows the aspects was woven. >> However, running the instrumented apache thrift lib gives this excetpion: >> >> (MyServer is my simple server implementation using thrift. >> TThreadPoolServer is the server class in apache thrift lib.) >> >> org.aspectj.lang.NoAspectBoundException >> at EpredPerRunnable.aspectOf(EpredPerRunnable.aj:1) >> at >> EpredRunnablesCallables.ajc$afterReturning$EpredRunnablesCallables$1$8a935d86(EpredRunnablesCallables.aj:55) >> at >> org.apache.thrift.server.TThreadPoolServer.serve(TThreadPoolServer.java:168) >> at MyServer.StartsimpleServer(MyServer.java:21) >> at MyServer.main(MyServer.java:28) >> >> My Aspects >> >> Here are the aspects I wrote: >> >> 1) I have a counter for each class implements Runnable using >> pertypewithin. >> >> privileged aspect PerRunnable >> pertypewithin(java.lang.Runnable+) >> { >> public long counter = 0; >> >> public long getCounter() { >> return counter; >> } >> >> public void incrementCounter() { >> counter++; >> } >> } >> >> 2) I insert an id into each class that implements Runnable using >> interface. >> >> privileged aspect MyRunnables { >> >> public interface InstrumentedRunnable {} >> >> private long InstrumentedRunnable.myid = -1; >> >> public long InstrumentedRunnable.getMyid() { >> return myid; >> } >> >> public void InstrumentedRunnable.setMyid(long id) { >> myid = id; >> } >> >> >> declare parents: (Runnable)+ implements InstrumentedRunnable; >> >> after() returning(InstrumentedRunnable r): >> call(java.lang.Runnable+.new(..)) { >> >> long id = PerRunnable.aspectOf(r.getClass()).getCounter(); >> r.setMyid(id); >> >> PerRunnable.aspectOf(r.getClass()).incrementCounter(); >> >> } >> >> } >> >> 3) Part of my scripts that only instruments thrift: >> >> CLASSPATH=$CLASSPATH:~/aspectj1.9/lib/aspectjtools.jar >> CLASSPATH=$CLASSPATH:~/aspectj1.9/lib/aspectjrt.jar >> AJC=~/aspectj1.9/bin/ajc >> >> echo "Compiling Aspects ..." >> $AJC -classpath $CLASSPATH:./lib/libthrift-0.11.0.jar -source 1.8 asp/*.aj >> >> echo "Weaving aspect into thrift lib..." >> $AJC -classpath >> $CLASSPATH:./lib/servlet-api-2.5.jar:./lib/httpcore-4.4.1.jar:./lib/slf4j-api-1.7.12.jar:./lib/httpclient-4.4.1.jar >> -source 1.8 -inpath ./lib/libthrift-0.11.0.jar -aspectpath ./asp/ -outjar >> ./my-libthrift-0.11.0.jar >> >> >> 4) Part of my scripts that starts the thrift server: >> >> CLASSPATH=$CLASSPATH:~/aspectj1.9/lib/aspectjtools.jar >> CLASSPATH=$CLASSPATH:~/aspectj1.9/lib/aspectjrt.jar >> >> java -cp >> $CLASSPATH:./asp:./my-add-server.jar:./my-libthrift-0.11.0.jar:./lib/slf4j-api-1.7.12.jar >> MyServer >> >> Need Help >> >> Has anyone met such problem before? Any guess? Note that these aspects >> works for my own Runnable but not for thrift lib. (I can send more code >> needed including my test classes and my scripts, but they don’t fit within >> an email…) >> Is there a way to get all aspect instances and what they are matched to at >> runtime? >> Does aspectj has this feature: given 1) a pointcut, 2) the signature of a >> target (class/method) I want the pointcut to match, tell me whether they >> matched, and if not why. >> >> Thank you for your time and help! >> >> >> >> >> >> >> >> >> >> _______________________________________________ >> aspectj-users mailing list >> aspectj-users@eclipse.org >> To change your delivery options, retrieve your password, or unsubscribe >> from this list, visit >> https://dev.eclipse.org/mailman/listinfo/aspectj-users > > _______________________________________________ > aspectj-users mailing list > aspectj-users@eclipse.org > To change your delivery options, retrieve your password, or unsubscribe from > this list, visit > https://dev.eclipse.org/mailman/listinfo/aspectj-users > > > > > _______________________________________________ > aspectj-users mailing list > aspectj-users@eclipse.org > To change your delivery options, retrieve your password, or unsubscribe from > this list, visit > https://dev.eclipse.org/mailman/listinfo/aspectj-users -- Yongle Zhang Web: http://yonglezh.com/ Department of Electrical and Computer Engineering, University of Toronto, D.L. Pratt Building 372, 6 King's College Road, Toronto, M5S 3H5 _______________________________________________ aspectj-users mailing list aspectj-users@eclipse.org To change your delivery options, retrieve your password, or unsubscribe from this list, visit https://dev.eclipse.org/mailman/listinfo/aspectj-users
