Some of my earlier changes made it safe to use public variables instead of getters/setters, at least with the compiler defaults. With certain combinations of compiler options, it is possible to make it unsafe again, though.
If you decided to use -export-public-symbols=true and -prevent-rename-public-symbols=false together, for some reason (this wouldn't be common), that would make public variables unsafe to use again. Closure compiler would make a separate copy of the variable with that combination, which was the original issue that caused us to add the warning. A copy is not made for getters and setters, so that's why that advice became common. When you prevent renaming, Closure is forced to use the original variable when exporting, so there is no copy, and it's safe. -- Josh Tynjala Bowler Hat LLC <https://bowlerhat.dev> On Mon, May 24, 2021 at 11:07 PM Yishay Weiss <yishayj...@hotmail.com> wrote: > Hi Josh, > > I still haven’t wrapped my head around all of this but thanks for the > informative post. Does this somehow mean public vars are now always safe to > use as opposed to having to covert them to public methods? > > From: Josh Tynjala<mailto:joshtynj...@bowlerhat.dev> > Sent: Monday, May 24, 2021 11:32 PM > To: Apache Royale Development<mailto:dev@royale.apache.org> > Subject: Compiler options for reducing release build size of Royale > projects (Part 2) > > Back in November, I shared a summary of some of the work that I've been > doing on reducing the size of release builds generated by the Royale > compiler. Essentially, I've been trying to ensure that we could take better > advantage of Closure compiler's advanced optimizations for minification, > which includes removing dead code and renaming symbols so that they have > shorter names. > > Since then, I've made some more improvements to the Royale compiler, which > will allow even more optimization. I figured that it is a good time to > share my latest results. > > To fully understand the context of today's post, I recommend reviewing the > introduction that I wrote for my earlier post (you can stop once I start > talking about the results). Here's the link to the archive: > > > https://lists.apache.org/thread.html/rf53d200969d795a15bda5d6635b904d89fa7f6af151df5dca15ccbbb%40%3Cdev.royale.apache.org%3E > > Last time, in my final set of results, some public symbols were allowed to > be renamed in the release build, but some were not. If I tried to rename > all public symbols, it would cause parts of the apps to stop working > correctly. My work since then has been finding ways to ensure that more > public symbols can be renamed safely. > > What I determined is that the way that we encode MXML data in JavaScript > results in a lot of dynamically setting properties using string names, > similar to `variable["propName"] = value`. Setting properties dynamically > is generally bad when minimizing with Closure compiler because it won't > understand that it needs to keep those properties, and it may remove them > as dead code (even though it's not dead code), or it may change their names > (while our code still uses the original names). > > Luckly, our MXML data in JS is well-structured, so it's possible to walk > the JS tree when creating the release build and extract names that are used > dynamically in MXML. Then, we can pass those names to Closure compiler > manually. Basically, we're extending the Closure compiler with an extra > analysis step that has specialized knowledge of Royale. This allows us to > rename more public symbols because we can exclude the ones that we know > cannot be renamed). > > I added a new compiler option, *prevent-rename-mxml-symbol-references*, > which defaults to true. You will probably never want to change this to > false, but the option is there, if anyone ever determines that it is needed > in some edge case. > > Okay, now to the results. I built the same four app projects as last time. > > I used the following commits, if anyone wants to try reproducing on their > own computer: > > royale-compiler: 65805281ea898b3a0225922f5956c864f810c423 > royale-asjs: fab5f5444e0e9ee7f8f59e5cf1d4b1144a31025e > spectrum-royale: acf4007fa322d61e1d0ae874e4c2009f8645654c > > ========== > Baseline: royale-compiler 0.9.7 > ========== > > HelloWorld: 71 KB > ASDoc: 237 KB > TourDeJewel: 1125 KB > SpectrumBrowser: 944 KB > > As with my previous post, I built all of the example projects with the > Royale 0.9.7 compiler first. However, I built them with the latest > framework code. This ensures that only the differences in the compiler are > being compared, and we don't have extra noise from differences in the > framework code. > > ----- > > Now, let's see some results with the latest compiler. > > All results below include the +/- difference in KB and %. These values are > always in comparison to the baseline numbers above. > > ========== > Result 1: Latest compiler with default options > ========== > > HelloWorld: 87 KB (+16 KB / +23%) > ASDoc: 261 KB (+24 KB / +10%) > TourDeJewel: 1159 KB (+34 KB / +3%) > SpectrumBrowser: 980 KB (+36 KB / +4%) > > As with the previous post, the increase in file size here is expected. The > latest compiler fixes some bugs that were present in 0.9.7 that caused > certain symbols to be renamed/removed when they should not have been. > > ========== > Result 2: Disable symbol exports and allow non-public symbols to be renamed > ========== > > HelloWorld: 74 KB (+3 KB / +4%) > ASDoc: 226 KB (-11 KB / -5%) > TourDeJewel: 966 KB (-159 KB / -14%) > SpectrumBrowser: 897 KB (-47 KB / -5%) > > Compiler options used: > *-export-public-symbols=false* > *-prevent-rename-protected-symbols=false* > *-prevent-rename-internal-symbols=false* > > These results are consistent with a similar test from my previous post. I > included it to show that nothing has changed, and to have an intermediate > step between the default options and the full optimizations. > > ========== > Result 3: Allow public symbols to be renamed too > ========== > > HelloWorld: 66 KB (-5 KB / -7%) > ASDoc: N/A > TourDeJewel: N/A > SpectrumBrowser: 857 KB (-87 KB / -9%) > > Compiler options used: > -export-public-symbols=false > -prevent-rename-protected-symbols=false > -prevent-rename-internal-symbols=false > *-prevent-rename-public-symbols=false* > > This is a compiler option that I didn't include in the tests last time > because renaming all public symbols broke every app before. Now that the > Royale compiler walks the MXML data tree, it's possible to allow renaming > of public symbols in some apps (but not all, for reasons I will explain). > *This > combination of compiler options provides the largest possible file size > savings in release builds.* > > You'll notice that ASDoc and TourDeJewel are marked N/A. This is because > they use the ConstantBinding class, which sets up bindings dynamically. > > <js:ConstantBinding sourceID="listModel" sourcePropertyName="iconListData" > destinationPropertyName="dataProvider"/> > > When compiled to JS, this code basically converts to this: > > this[binding.destinationPropertyName] = > this[binding.sourceID][binding.sourcePropertyName]; > > Notice the bracket access with string values. Like I explained before, > this is something that Closure compiler cannot detect. > > Technically, I could write another custom Closure compiler pass that looks > for ConstantBinding objects in the MXML data tree and extract the property > names. It wouldn't be hard, but what happens if new types of bindings are > added that work similarly? Do I just keep adding them to the compiler as > special cases? Or what if someone wants to use the compiler with a > different framework and wants to move ConstantBinding to a different > package? Do they need to modify the compiler too? It simply doesn't feel > clean to me. So, if you want to use the > -prevent-rename-public-symbols=false compiler option, I recommend avoiding > ConstantBinding. Regular bindings with curly braces {} are fully supported, > and they're what most people will be familiar with from Flex anyway. > > ========== > Extra Result: Allow *some* public symbols to be renamed > ========== > > ASDoc: 210 KB (-27 KB / -11%) > TourDeJewel: 927 KB (-198 KB / -18%) > > Compiler options used: > -export-public-symbols=false > -prevent-rename-protected-symbols=false > -prevent-rename-internal-symbols=false > *-prevent-rename-public-static-methods=false* > *-prevent-rename-public-instance-methods=false* > > That being said, even when using ConstantBinding or other dynamic features, > it may be possible to rename a subset of public symbols. Most commonly, > public methods are usually safe to rename because most people don't try to > call methods dynamically. This final extra test is consistent with a > similar result from my previous post. > > That was really long, so thank you for reading, if you made it to the end! > > -- > Josh Tynjala > Bowler Hat LLC <https://bowlerhat.dev> > >
