Community
Participate
Working Groups
In order to facilitate widespread use of LTW we need to ensure code is compiled with –Xreweavable. However we cannot rely on developers to use this flag so it must be the default. Unfortunately the current code bloat is unacceptable for large projects.
Should try and look at this prior to 1.5.0 shipping. Currently we have normal reweavable which keeps a copy of the pre-woven class file, or reweavable:compress which keeps a zipped (using ZipStream) copy of the pre-woven class file. We need to look at some binary diff tools, and either: - Just do a diff between the class bytes pre and post weave. - Use more knowledge that we are diffing class files and perhaps process different areas (constant pool, code attributes) differently.
See WeaverStateInfo.readAnyReweavableData and WeaverStateInfo.writeAnyReweavableData. That's where the improvements should go - toughest bit will be getting hold of the bytes representing the finally woven class as that must include everything *except* the WeaverStateInfo attribute where the delta will be stored.
Numbers and stuff ================= I've made two additional versions of -Xreweavable:compress, one which stores the DIFF of the unwoven file and one which stores the COMPRESSED DIFF of the unwoven file. SIZE ON DISK ============ PROJECT SIZE ZIPPED JAR SIZE WOVEN 100% 48% REWEAVABLE 152% 58% REWEAVABLE:COMPRESSED 125% 75% REWEAVABLE:COMPRESSED DIFF 110% 59% REWEAVABLE:DIFF 115% 56% These show some interesting results: compiling using reweavable:compressed actually _increases_ the size of the outjar considerably! This is because the second compression can't see that the zipped part of reweavable files is very similar when unzipped to the non-zipped part of the reweavable files. This means that reweavable:diff is the best overall, as people who care about disk space usage will (should) be compiling to an outjar. So overall: From woven (not reweavable) to woven:reweavable using an outjar, the increase in size was 120% And now, from woven (not reweavable) to woven:reweavable (using diff) using an outjar, the increase in size is 117% So, I'm going to clean up my code a bit, add some unit tests and then perhaps make "uncompressed diff" the default.
Note, the project I used to get this data on consisted of 77 source files, 72 of which were touched by aspects.
If I'm going to make the uncompressed diff thing the default, then -Xreweavable and -Xreweavable:compress will become depricated.. and perhaps a couple of extra -X options should be added: -Xreweavable:noDiff and -XnoReweavable
What is the performance impact to the build time for the different modes?
reweavable took 8.54 seconds reweavable:compress took 8.69 seconds and reweavable with the diff took 9.68 seconds given that the test-project had 77 files, diffing it would be 13.68 seconds longer than just doing it reweavable on a 1000 file project. Thats too long, isn't it?
How long does it take to compile that program (77 files) with *no* reweavable option specified? It's not necessarily a problem that building 1000 file project takes 13 seconds longer, since we will hopefully normally be doing incremental compiles for single file changes, and as long as that remains near instant (<1second) then we may be OK. I'm just trying to decide the right conditions under which to make reweavable the default. Alex has already changed AJ so that aspects are always built reweavable - the question is over what to do for normal classes...
I've just made a new way of diffing the file, which I'm calling "DumDiff" SIZE ON DISK ============ PROJECT SIZE ZIPPED JAR SIZE WOVEN 100% 48% REWEAVABLE 152% 58% REWEAVABLE:DIFF 115% 56% REWEAVABLE:DUMDIFF 122% 55% DumDiff is incredibly fast (at least ten times as fast as diff) and it produces the smallest zipped-jar size... so now I think DumDiff should become the default.
the dumdiff reweavable method makes the project 114% of the size it would be non-reweavable.
We should also remove -noweave at the same time because it is evil and will no longer be necessary.
Created attachment 27338 [details] partly complete patch Alright, having found that months ago I accidentally deleted the wrong workspace, I've had to rewrite the diff thing I had. The good news is the new version works and is better than the old one was :) I'm including a patch here of what I've got so far just incase my computer explodes.. it currently works the same as normal, but when -Xreweavable:compress is specified it uses my diff thing.... I'll put another patch here soonish which will be back compatible with files made with the old -Xreweavable:compress, and which will have all the options wired up correctly, the new options will be: (nothing) the default makes classfiles reweavable using my diff thing -Xreweavable the same as the default, but outputs a warning -Xreweavable:compress the same as the default, but outputs a warning -Xnoweave gives an error -XnotReweavable makes non-reweavable classfiles people should tell me now if they want the options to do different things!
dont change -Xnoweave!! I know we don't like it but I use it all the time for testing.
acknowledged
Created attachment 27436 [details] complete patch This zipped patch is synchronised and passes all the run-these-before-you-commit tests.
assigning to andy...
reminding myself that this is likely to break incremental compilation, watch out!
Andrew, I can't apply the entire patch (bloody linux). I need you to sync and recreate the patches *only* for the org.aspectj.ajdt.core module and the loadtime module. The others went on OK.
Created attachment 27560 [details] loadtime and org.aspectj.ajdt.core again don't know if this'll work any better :-/
hmmmmm - did you sync before making those new diffs? they don't seem to take account of -Xdev:pinpoint which is in the argument parsing logic now.... i'm trying to hack them in regardless but it may go wrong.
Created attachment 27614 [details] one more time damn whitespace!
This feature is about to go in. I've been tweaking it a bit today and had to change some option handling slightly so that it worked with some of the other recent changes that have gone into CVS. I thought I'd try it out building the biggest AJ project we have, Shadows. -XnotReweavable (default reweavable) time to build ~41s ~41s Size of bin directory after build 8532463bytes 9198561bytes Zipped bin directory after build 3760391bytes 3949184bytes There are >1200 files in shadows. Average impact is about 500bytes on each 7000byte class file, which is <10% Comparing this with the old reweavable implementation: Xreweavable: 9960142 bin dir, 4036844 zipped. Xreweavable:compress: 9107662 bin dir, 4358636 zipped. I am going to tag the tree before checking it in ... just in case ;)
well ... i've done it, this feature is now integrated into CVS. it will be interesting to see how well it behaves with AJDT.
reweavable is default *gulp*
I strongly suspect we should make -XnotReweavable the default for LTW as is no doubt more expensive and redundant in that mode.
