You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@royale.apache.org by Harbs <ha...@gmail.com> on 2019/02/04 09:57:44 UTC

New minification problem

TLF has the following code in SpanElement:


		/** @private */
		override public function createContentElement():void
		{
			if (_blockElement && _blockElement.textBlock)
				return;
			
			calculateComputedFormat();	// BEFORE creating the element
			_blockElement = new TextElement(_text,null);			
			super.createContentElement();
		}

Which gets compiled to this:

/** @asprivate 
 * @export
 * @override
 */
org.apache.royale.textLayout.elements.SpanElement.prototype.createContentElement = function() {
  if (this._blockElement && this._blockElement.textBlock)
    return;
  this.calculateComputedFormat();
  this._blockElement = new com.printui.text.engine.TextElement(this._text, null);
  
  
  org.apache.royale.textLayout.elements.SpanElement.superClass_.createContentElement.apply(this);
};

Minified, that function looks like this:
hK.prototype.sf = function() {
    this.Xa && this.Xa.textBlock || (this.Xa = new iK(this._text,null),
    hK.o.sf.apply(this))
}

Notice that the function call of calculateComputedFormat(); is missing in the minified code.

The reason for this is because the function is deemed to not have side effects by the Closure compiler.

That function (in FlowElement) looks like this:
		public function get computedFormat():ITextLayoutFormat
		{
			if (_computedFormat == null)
				_computedFormat = doComputeTextLayoutFormat();
			return _computedFormat;
		}

		public function calculateComputedFormat():ITextLayoutFormat
		{
			// use the class-sppecific getter
			return computedFormat;
		}
Apparently, the Closure Compiler assumes that getters do not have side effects.

I can work around this problem in TLF, but it seems to me that this is a problem which should be fixed in the compiler.

Thoughts?
Harbs


Re: New minification problem

Posted by Alex Harui <ah...@adobe.com.INVALID>.
There are probably other interesting code patterns that will break under ADVANCED_OPTIMIZATIONS.  IMO, we don't have to make every pattern survive advanced optimizations, just good coding patterns.  Google will probably get around to fixing their optimization someday.

Related, in trying to get Royale modules to work like Flex modules under advanced optimization, I found that we have to find ways to force protected methods in the main app from getting removed if they are only used in the module.  In a SWF, all methods are there just-in-case.  Not so in JS.  With limited committer resources, I don't think we should solve all of these issues in the compiler right now.  There are bigger fish to fry, IMO.

The change in the compiler from calling get_/set_ functions to using getter/setters was significant.  I would not want to support both code paths if we can avoid it.  It even changed where in the emitter you could emit something because you had to remember that a set expression was going to require parameters instead of just emitting the properties in the order found.  Expressions like:

  a++;

And

  a += "foo";

required special handling.

One thing you could try is adding compiler options to control renaming.  We've mentioned shrinking the names of private variables.  You could add that as well as an option to rename getter/setters.

You could also experiment with an option to emit simple get expressions as the get_ function.  That should be cheap and easy.  Setting stuff is where the big changes would happen.  Some complex get_ expressions might also need to be avoided in order to avoid having different tree-walks based on the option.  Getting everything to not use getter/setters would be too disruptive.  Maybe see how far you can get with "some".

Related, I came upon an article about the Google Closure Compiler saying that the output is optimized for ZIP transfer over the network.  And thus, the true test of output size is a zip of the JS file, not the actual JS file.  Google said that everyone should be using zip compression these days.  Are you?  The Zip compression may replace all of those occurrences of the getter string anyway.

HTH,-
Alex




On 2/4/19, 2:12 PM, "Harbs" <ha...@gmail.com> wrote:

    Hmm. That *is* messy...
    
    I don’t really like the pattern very much myself, but breaking valid code isn’t ideal.
    
    The warning might be an acceptable compromise.
    
    Another problem I’m having with getters is that they are not minified. I wonder if there could be a compiler option to output the ugly functions to get better minification. I could see switching back and forth while debugging.
    
    > On Feb 4, 2019, at 8:39 PM, Alex Harui <ah...@adobe.com.INVALID> wrote:
    > 
    > The original output used functions and the output was really messy to read, so we switched to getter/setters.
    > 
    >   a = a + 1
    > 
    > transpiled to:
    > 
    >  set_a(get_a() + 1));
    > 
    > One option is that the transpiler should warn on unused gets.  That would at least flag places to look and refactor code.  I'm not sure how common unused gets are in existing AS code.  TLF used that pattern in several places, but I'm not sure about elsewhere.  I don't like the pattern.  It relies on the maintainer remembering that there are side-effects.
    > 
    > My 2 cents,
    > -Alex
    > 
    > On 2/4/19, 9:43 AM, "Harbs" <ha...@gmail.com> wrote:
    > 
    >    I’m not sure.
    > 
    >    One thought I had (which might help get better minified code in general) is to use the getter functions rather than the getters themselves. i.e. in this case output "return get__computedFormat()" instead of "return computedFormat”.
    > 
    > 
    >> On Feb 4, 2019, at 6:52 PM, Alex Harui <ah...@adobe.com.INVALID> wrote:
    >> 
    >> That is the correct analysis.  What do you propose the compiler do differently?
    >> 
    >> -Alex
    >> 
    >> On 2/4/19, 1:57 AM, "Harbs" <ha...@gmail.com> wrote:
    >> 
    >>   TLF has the following code in SpanElement:
    >> 
    >> 
    >>   		/** @private */
    >>   		override public function createContentElement():void
    >>   		{
    >>   			if (_blockElement && _blockElement.textBlock)
    >>   				return;
    >>   			
    >>   			calculateComputedFormat();	// BEFORE creating the element
    >>   			_blockElement = new TextElement(_text,null);			
    >>   			super.createContentElement();
    >>   		}
    >> 
    >>   Which gets compiled to this:
    >> 
    >>   /** @asprivate 
    >>    * @export
    >>    * @override
    >>    */
    >>   org.apache.royale.textLayout.elements.SpanElement.prototype.createContentElement = function() {
    >>     if (this._blockElement && this._blockElement.textBlock)
    >>       return;
    >>     this.calculateComputedFormat();
    >>     this._blockElement = new com.printui.text.engine.TextElement(this._text, null);
    >> 
    >> 
    >>     org.apache.royale.textLayout.elements.SpanElement.superClass_.createContentElement.apply(this);
    >>   };
    >> 
    >>   Minified, that function looks like this:
    >>   hK.prototype.sf = function() {
    >>       this.Xa && this.Xa.textBlock || (this.Xa = new iK(this._text,null),
    >>       hK.o.sf.apply(this))
    >>   }
    >> 
    >>   Notice that the function call of calculateComputedFormat(); is missing in the minified code.
    >> 
    >>   The reason for this is because the function is deemed to not have side effects by the Closure compiler.
    >> 
    >>   That function (in FlowElement) looks like this:
    >>   		public function get computedFormat():ITextLayoutFormat
    >>   		{
    >>   			if (_computedFormat == null)
    >>   				_computedFormat = doComputeTextLayoutFormat();
    >>   			return _computedFormat;
    >>   		}
    >> 
    >>   		public function calculateComputedFormat():ITextLayoutFormat
    >>   		{
    >>   			// use the class-sppecific getter
    >>   			return computedFormat;
    >>   		}
    >>   Apparently, the Closure Compiler assumes that getters do not have side effects.
    >> 
    >>   I can work around this problem in TLF, but it seems to me that this is a problem which should be fixed in the compiler.
    >> 
    >>   Thoughts?
    >>   Harbs
    >> 
    >> 
    >> 
    > 
    > 
    > 
    
    


Re: New minification problem

Posted by Harbs <ha...@gmail.com>.
Hmm. That *is* messy...

I don’t really like the pattern very much myself, but breaking valid code isn’t ideal.

The warning might be an acceptable compromise.

Another problem I’m having with getters is that they are not minified. I wonder if there could be a compiler option to output the ugly functions to get better minification. I could see switching back and forth while debugging.

> On Feb 4, 2019, at 8:39 PM, Alex Harui <ah...@adobe.com.INVALID> wrote:
> 
> The original output used functions and the output was really messy to read, so we switched to getter/setters.
> 
>   a = a + 1
> 
> transpiled to:
> 
>  set_a(get_a() + 1));
> 
> One option is that the transpiler should warn on unused gets.  That would at least flag places to look and refactor code.  I'm not sure how common unused gets are in existing AS code.  TLF used that pattern in several places, but I'm not sure about elsewhere.  I don't like the pattern.  It relies on the maintainer remembering that there are side-effects.
> 
> My 2 cents,
> -Alex
> 
> On 2/4/19, 9:43 AM, "Harbs" <ha...@gmail.com> wrote:
> 
>    I’m not sure.
> 
>    One thought I had (which might help get better minified code in general) is to use the getter functions rather than the getters themselves. i.e. in this case output "return get__computedFormat()" instead of "return computedFormat”.
> 
> 
>> On Feb 4, 2019, at 6:52 PM, Alex Harui <ah...@adobe.com.INVALID> wrote:
>> 
>> That is the correct analysis.  What do you propose the compiler do differently?
>> 
>> -Alex
>> 
>> On 2/4/19, 1:57 AM, "Harbs" <ha...@gmail.com> wrote:
>> 
>>   TLF has the following code in SpanElement:
>> 
>> 
>>   		/** @private */
>>   		override public function createContentElement():void
>>   		{
>>   			if (_blockElement && _blockElement.textBlock)
>>   				return;
>>   			
>>   			calculateComputedFormat();	// BEFORE creating the element
>>   			_blockElement = new TextElement(_text,null);			
>>   			super.createContentElement();
>>   		}
>> 
>>   Which gets compiled to this:
>> 
>>   /** @asprivate 
>>    * @export
>>    * @override
>>    */
>>   org.apache.royale.textLayout.elements.SpanElement.prototype.createContentElement = function() {
>>     if (this._blockElement && this._blockElement.textBlock)
>>       return;
>>     this.calculateComputedFormat();
>>     this._blockElement = new com.printui.text.engine.TextElement(this._text, null);
>> 
>> 
>>     org.apache.royale.textLayout.elements.SpanElement.superClass_.createContentElement.apply(this);
>>   };
>> 
>>   Minified, that function looks like this:
>>   hK.prototype.sf = function() {
>>       this.Xa && this.Xa.textBlock || (this.Xa = new iK(this._text,null),
>>       hK.o.sf.apply(this))
>>   }
>> 
>>   Notice that the function call of calculateComputedFormat(); is missing in the minified code.
>> 
>>   The reason for this is because the function is deemed to not have side effects by the Closure compiler.
>> 
>>   That function (in FlowElement) looks like this:
>>   		public function get computedFormat():ITextLayoutFormat
>>   		{
>>   			if (_computedFormat == null)
>>   				_computedFormat = doComputeTextLayoutFormat();
>>   			return _computedFormat;
>>   		}
>> 
>>   		public function calculateComputedFormat():ITextLayoutFormat
>>   		{
>>   			// use the class-sppecific getter
>>   			return computedFormat;
>>   		}
>>   Apparently, the Closure Compiler assumes that getters do not have side effects.
>> 
>>   I can work around this problem in TLF, but it seems to me that this is a problem which should be fixed in the compiler.
>> 
>>   Thoughts?
>>   Harbs
>> 
>> 
>> 
> 
> 
> 


Re: New minification problem

Posted by Alex Harui <ah...@adobe.com.INVALID>.
The original output used functions and the output was really messy to read, so we switched to getter/setters.

   a = a + 1

transpiled to:

  set_a(get_a() + 1));

One option is that the transpiler should warn on unused gets.  That would at least flag places to look and refactor code.  I'm not sure how common unused gets are in existing AS code.  TLF used that pattern in several places, but I'm not sure about elsewhere.  I don't like the pattern.  It relies on the maintainer remembering that there are side-effects.

My 2 cents,
-Alex

On 2/4/19, 9:43 AM, "Harbs" <ha...@gmail.com> wrote:

    I’m not sure.
    
    One thought I had (which might help get better minified code in general) is to use the getter functions rather than the getters themselves. i.e. in this case output "return get__computedFormat()" instead of "return computedFormat”.
    
    
    > On Feb 4, 2019, at 6:52 PM, Alex Harui <ah...@adobe.com.INVALID> wrote:
    > 
    > That is the correct analysis.  What do you propose the compiler do differently?
    > 
    > -Alex
    > 
    > On 2/4/19, 1:57 AM, "Harbs" <ha...@gmail.com> wrote:
    > 
    >    TLF has the following code in SpanElement:
    > 
    > 
    >    		/** @private */
    >    		override public function createContentElement():void
    >    		{
    >    			if (_blockElement && _blockElement.textBlock)
    >    				return;
    >    			
    >    			calculateComputedFormat();	// BEFORE creating the element
    >    			_blockElement = new TextElement(_text,null);			
    >    			super.createContentElement();
    >    		}
    > 
    >    Which gets compiled to this:
    > 
    >    /** @asprivate 
    >     * @export
    >     * @override
    >     */
    >    org.apache.royale.textLayout.elements.SpanElement.prototype.createContentElement = function() {
    >      if (this._blockElement && this._blockElement.textBlock)
    >        return;
    >      this.calculateComputedFormat();
    >      this._blockElement = new com.printui.text.engine.TextElement(this._text, null);
    > 
    > 
    >      org.apache.royale.textLayout.elements.SpanElement.superClass_.createContentElement.apply(this);
    >    };
    > 
    >    Minified, that function looks like this:
    >    hK.prototype.sf = function() {
    >        this.Xa && this.Xa.textBlock || (this.Xa = new iK(this._text,null),
    >        hK.o.sf.apply(this))
    >    }
    > 
    >    Notice that the function call of calculateComputedFormat(); is missing in the minified code.
    > 
    >    The reason for this is because the function is deemed to not have side effects by the Closure compiler.
    > 
    >    That function (in FlowElement) looks like this:
    >    		public function get computedFormat():ITextLayoutFormat
    >    		{
    >    			if (_computedFormat == null)
    >    				_computedFormat = doComputeTextLayoutFormat();
    >    			return _computedFormat;
    >    		}
    > 
    >    		public function calculateComputedFormat():ITextLayoutFormat
    >    		{
    >    			// use the class-sppecific getter
    >    			return computedFormat;
    >    		}
    >    Apparently, the Closure Compiler assumes that getters do not have side effects.
    > 
    >    I can work around this problem in TLF, but it seems to me that this is a problem which should be fixed in the compiler.
    > 
    >    Thoughts?
    >    Harbs
    > 
    > 
    > 
    
    


Re: New minification problem

Posted by Harbs <ha...@gmail.com>.
I’m not sure.

One thought I had (which might help get better minified code in general) is to use the getter functions rather than the getters themselves. i.e. in this case output "return get__computedFormat()" instead of "return computedFormat”.


> On Feb 4, 2019, at 6:52 PM, Alex Harui <ah...@adobe.com.INVALID> wrote:
> 
> That is the correct analysis.  What do you propose the compiler do differently?
> 
> -Alex
> 
> On 2/4/19, 1:57 AM, "Harbs" <ha...@gmail.com> wrote:
> 
>    TLF has the following code in SpanElement:
> 
> 
>    		/** @private */
>    		override public function createContentElement():void
>    		{
>    			if (_blockElement && _blockElement.textBlock)
>    				return;
>    			
>    			calculateComputedFormat();	// BEFORE creating the element
>    			_blockElement = new TextElement(_text,null);			
>    			super.createContentElement();
>    		}
> 
>    Which gets compiled to this:
> 
>    /** @asprivate 
>     * @export
>     * @override
>     */
>    org.apache.royale.textLayout.elements.SpanElement.prototype.createContentElement = function() {
>      if (this._blockElement && this._blockElement.textBlock)
>        return;
>      this.calculateComputedFormat();
>      this._blockElement = new com.printui.text.engine.TextElement(this._text, null);
> 
> 
>      org.apache.royale.textLayout.elements.SpanElement.superClass_.createContentElement.apply(this);
>    };
> 
>    Minified, that function looks like this:
>    hK.prototype.sf = function() {
>        this.Xa && this.Xa.textBlock || (this.Xa = new iK(this._text,null),
>        hK.o.sf.apply(this))
>    }
> 
>    Notice that the function call of calculateComputedFormat(); is missing in the minified code.
> 
>    The reason for this is because the function is deemed to not have side effects by the Closure compiler.
> 
>    That function (in FlowElement) looks like this:
>    		public function get computedFormat():ITextLayoutFormat
>    		{
>    			if (_computedFormat == null)
>    				_computedFormat = doComputeTextLayoutFormat();
>    			return _computedFormat;
>    		}
> 
>    		public function calculateComputedFormat():ITextLayoutFormat
>    		{
>    			// use the class-sppecific getter
>    			return computedFormat;
>    		}
>    Apparently, the Closure Compiler assumes that getters do not have side effects.
> 
>    I can work around this problem in TLF, but it seems to me that this is a problem which should be fixed in the compiler.
> 
>    Thoughts?
>    Harbs
> 
> 
> 


Re: New minification problem

Posted by Alex Harui <ah...@adobe.com.INVALID>.
That is the correct analysis.  What do you propose the compiler do differently?

-Alex

On 2/4/19, 1:57 AM, "Harbs" <ha...@gmail.com> wrote:

    TLF has the following code in SpanElement:
    
    
    		/** @private */
    		override public function createContentElement():void
    		{
    			if (_blockElement && _blockElement.textBlock)
    				return;
    			
    			calculateComputedFormat();	// BEFORE creating the element
    			_blockElement = new TextElement(_text,null);			
    			super.createContentElement();
    		}
    
    Which gets compiled to this:
    
    /** @asprivate 
     * @export
     * @override
     */
    org.apache.royale.textLayout.elements.SpanElement.prototype.createContentElement = function() {
      if (this._blockElement && this._blockElement.textBlock)
        return;
      this.calculateComputedFormat();
      this._blockElement = new com.printui.text.engine.TextElement(this._text, null);
      
      
      org.apache.royale.textLayout.elements.SpanElement.superClass_.createContentElement.apply(this);
    };
    
    Minified, that function looks like this:
    hK.prototype.sf = function() {
        this.Xa && this.Xa.textBlock || (this.Xa = new iK(this._text,null),
        hK.o.sf.apply(this))
    }
    
    Notice that the function call of calculateComputedFormat(); is missing in the minified code.
    
    The reason for this is because the function is deemed to not have side effects by the Closure compiler.
    
    That function (in FlowElement) looks like this:
    		public function get computedFormat():ITextLayoutFormat
    		{
    			if (_computedFormat == null)
    				_computedFormat = doComputeTextLayoutFormat();
    			return _computedFormat;
    		}
    
    		public function calculateComputedFormat():ITextLayoutFormat
    		{
    			// use the class-sppecific getter
    			return computedFormat;
    		}
    Apparently, the Closure Compiler assumes that getters do not have side effects.
    
    I can work around this problem in TLF, but it seems to me that this is a problem which should be fixed in the compiler.
    
    Thoughts?
    Harbs