[FlexJS] hard coded event names

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[FlexJS] hard coded event names

Justin Mclean
Administrator
Hi,

In the Flex SDK we have a large number of event names hard coded. Is there any reason (performance or otherwise) for coding in this style?

Usually I would define them as consts in a single place and refer to that. Are we assuming the compiler will optimise this and that no one will miss type event names?

Thanks,
Justin
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [FlexJS] hard coded event names

yishayw
I think the idea is to optimize the number of different event classes. Event and ValueEvent should take care of a lot of cases. If you add a bunch of constants to those 2, you end up bloating them. App developers should get code hinting in mxml via metadata so typos are mitigated.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [FlexJS] hard coded event names

Justin Mclean
Administrator
Hi,

> If you add a bunch of constants to those 2, you end up bloating them.

Still seems a higher cost to have the same thing mentioned dozens of times. Not measured the exact performance impact however.

> App developers should get code hinting in mxml via metadata so typos are mitigated.

I tend to find I use most event names (other than obvious ones like click handlers) in AS code not MXML.

Thanks,
Justin
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [FlexJS] hard coded event names

Christofer Dutz
Hi Justin,

Didn’t in the past most Event types contain the constants inside themselves?

Chris

Am 23.05.17, 12:14 schrieb "Justin Mclean" <[hidden email]>:

    Hi,
   
    > If you add a bunch of constants to those 2, you end up bloating them.
   
    Still seems a higher cost to have the same thing mentioned dozens of times. Not measured the exact performance impact however.
   
    > App developers should get code hinting in mxml via metadata so typos are mitigated.
   
    I tend to find I use most event names (other than obvious ones like click handlers) in AS code not MXML.
   
    Thanks,
    Justin

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [FlexJS] hard coded event names

Justin Mclean
Administrator
Hi,

> Didn’t in the past most Event types contain the constants inside themselves?

Yep in the Flex SDK that’s the case.

Thanks,
Justin
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [FlexJS] hard coded event names

Christofer Dutz
So, what was wrong with that practice?

I sort of got used to it and it makes maintaining and extending quite easy. I really dislike these constant-graves ;-)

Chris



Am 23.05.17, 12:56 schrieb "Justin Mclean" <[hidden email]>:

    Hi,
   
    > Didn’t in the past most Event types contain the constants inside themselves?
   
    Yep in the Flex SDK that’s the case.
   
    Thanks,
    Justin

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [FlexJS] hard coded event names

Harbs
In reply to this post by Justin Mclean
The current compiler setup makes use of static constants add considerably more bulk to the compiled code than string literals.

It might be possible to make the Google Compiler do a better job, but as it stands, we’re better off with string literals than constants.

The FlexJS style of defining constants are to define them where they are used rather than creating new Event classes simply for defining the constants. (i.e. Timer.TIMER rather than TimerEvent.TIMER)

> On May 23, 2017, at 5:21 AM, Justin Mclean <[hidden email]> wrote:
>
> Hi,
>
> In the Flex SDK we have a large number of event names hard coded. Is there any reason (performance or otherwise) for coding in this style?
>
> Usually I would define them as consts in a single place and refer to that. Are we assuming the compiler will optimise this and that no one will miss type event names?
>
> Thanks,
> Justin

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [FlexJS] hard coded event names

Alex Harui-2


On 5/23/17, 5:48 AM, "Harbs" <[hidden email]> wrote:
>
>The FlexJS style of defining constants are to define them where they are
>used rather than creating new Event classes simply for defining the
>constants. (i.e. Timer.TIMER rather than TimerEvent.TIMER)

This statement is a key difference that I hope will better address this
issue.  I'm not sure there is a 'right' answer.  I don't think this is a
release critical issue, but it is an interesting topic.

Long term, I hope we add inlining constants to the compiler.  Then we
might gravitate back towards more constants for event names, but in the
classes instead of new event classes.  AFAICT, strings optimize better
than constant lookups.  The tradeoff is that folks lose developer
productivity to mis-spelling or mis-capitalizing event names.  I don't
think there is enough pay-off to having lots of event classes with the
same or similar set of properties bloating the output.  It is pretty rare
to have to check the type of the Event.  In regular Flex the main
flash.events.Event class is way overloaded.  Similar for
mx.events.FlexEvent.  Just bringing it in brings in a bunch of potentially
unused constants.  I want to avoid that somehow in FlexJS.

Also, even in regular Flex there were two "unofficial" categories of
events: "public" events and "binding-only" events.  Many uses of string
event names in Flex and FlexJS are for binding-only events like
"textChanged".  It is rare to need to use those outside of binding
expressions.  Others are more commonly used from outside the class.
Having constants for those "public" events are useful for AS developers
but don't help the MXML developer.  In MXML you still have to type the
correct capitalization of the string event name.  IOW, you can use
Tooltip.TOOLTIP_CHANGED in AS but not in MXML.  You still have to look it
up to see if it is "tooltip" or "toolTip".  But the compiler and code
hinting should help you.  But we left binding-only events as strings as it
wasn't worth the cost of creating constants for everything, especially
these binding-only event name strings that are often one-offs

My 2 cents,
-Alex

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [FlexJS] hard coded event names

Justin Mclean
Administrator
Hi,

> I don't think this is a release critical issue, but it is an interesting topic.

Agreed not for this release anyway but coming up with a common pattern would be useful.

>  AFAICT, strings optimize better than constant lookups.

Have you done any tests to show this is the case?

>  The tradeoff is that folks lose developer productivity to mis-spelling or mis-capitalizing event names.

A single error here could cost multiple users a lot of time. Just wondering is there anther way it can be done safely?

>  I don’t think there is enough pay-off to having lots of event classes with the
> same or similar set of properties bloating the output.  It is pretty rare
> to have to check the type of the Event.

But you need the the event name in order to add/remove event listeners.

>  I want to avoid that somehow in FlexJS.

I would assume that the compile removes stuff that is not referenced if not at compile time, then at runtime. So possibly the only cost is in the size of the classes and run time lookup od statics. Having static strings would reduce the size of the classes so it seems it may be a site vs runtime cost. I’m guessing that runtime cost is very small but again not done any tests to see if that is actually the case.

Thanks,
Justin
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [FlexJS] hard coded event names

Harbs
Huh? Why would tests be necessary?

Like I already said; constants add much more bulk to the JS source code. Try it and you’ll see…

> On May 23, 2017, at 5:53 PM, Justin Mclean <[hidden email]> wrote:
>
> Have you done any tests to show this is the case?

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [FlexJS] hard coded event names

Greg Dove
Harbs, I think that is only for debug. I don't think they should add bulk
to the release code. At least they do not in the default settings for GCC
optimisation. You will see the definition of the constant only once and
inline replacement is used for the value (with a short name global
variable) everywhere. The only extra cost of a constant seems to be a
lookup reference that may be added. It seems quite minimal.

If you do a search for both the constant value e.g. 'change' or the the
name of the constant (e.g. FORM_SAVED ) in the release output you should
only ever see it once whichever way you do this.


On Wed, May 24, 2017 at 1:28 PM, Harbs <[hidden email]> wrote:

> Huh? Why would tests be necessary?
>
> Like I already said; constants add much more bulk to the JS source code.
> Try it and you’ll see…
>
> > On May 23, 2017, at 5:53 PM, Justin Mclean <[hidden email]>
> wrote:
> >
> > Have you done any tests to show this is the case?
>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [FlexJS] hard coded event names

Justin Mclean
Administrator
In reply to this post by Harbs
Hi,

> Like I already said; constants add much more bulk to the JS source code. Try it and you’ll see…

I’ve tried it and they don’t, but  perhaps I’m missing something?

Looking some AS code like so:

var result1:String = "One" + "Two" + "Three";
var result2:String = ONE + TWO + THREE;
var result3:String = Outside.ONE + Outside.TWO + Outside.THREE;
var duplicate1:String = "One" + "Two" + "Three";
var duplicate2:String = ONE + TWO + THREE;
var duplicate3:String = Outside.ONE + Outside.TWO + Outside.THREE;


On the debug side we get:

In Outside.js:
  Outside.ONE = "One”;
  Outside.TWO = "Two”;
  Outside.THREE = "Three”;

In main program:
  StaticConst.ONE = "One”;
  StaticConst.TWO = "Two”;
  StaticConst.THREE = "Three";

  var /** @type {string} */ result1 = "One" + "Two" + "Three";
  var /** @type {string} */ result2 = StaticConst.ONE + StaticConst.TWO + StaticConst.THREE;
  var /** @type {string} */ result3 = Outside.ONE + Outside.TWO + Outside.THREE;
  var /** @type {string} */ duplicate1 = "One" + "Two" + "Three";
  var /** @type {string} */ duplicate2 = StaticConst.ONE + StaticConst.TWO + StaticConst.THREE;
  var /** @type {string} */ duplicate3 = Outside.ONE + Outside.TWO + Outside.THREE;

There doesn’t seem to be any optimisation in any of the three methods for debug and in fact using strings vs static const looks to increases the memory use of the code as you have multiple instances of “one”, “two”, “three”.It may do some optimisation at runtime I guess. But at a casual glance hard coding duplicate strings seem worse off although I agree it doesn’t have the object property lookup cost. I can’t see that would be significant.

For release we get all three get treated exactly same way as Greg pointed out. In fact it goes as far as to optimise them all to be the same variable and use that single variable rather than have 6 different variables.

So I can’t really see any reason for hard coding this in the SDK as it seems you get exactly the same results in production and probably better off in debug.

Thanks,
Justin
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [FlexJS] hard coded event names

Alex Harui-2
Again, interesting topic, but maybe table it until after the release?

I was under the assumption that every constant adds an alias to the
output.  Otherwise data binding to constants would not work after variable
renaming.  But again, I'd prefer that we don't spend time on this topic
until after the release.

Thanks,
-Alex

On 5/23/17, 8:14 PM, "Justin Mclean" <[hidden email]> wrote:

>Hi,
>
>> Like I already said; constants add much more bulk to the JS source
>>code. Try it and you’ll see…
>
>I’ve tried it and they don’t, but  perhaps I’m missing something?
>
>Looking some AS code like so:
>
>var result1:String = "One" + "Two" + "Three";
>var result2:String = ONE + TWO + THREE;
>var result3:String = Outside.ONE + Outside.TWO + Outside.THREE;
>var duplicate1:String = "One" + "Two" + "Three";
>var duplicate2:String = ONE + TWO + THREE;
>var duplicate3:String = Outside.ONE + Outside.TWO + Outside.THREE;
>
>
>On the debug side we get:
>
>In Outside.js:
>  Outside.ONE = "One”;
>  Outside.TWO = "Two”;
>  Outside.THREE = "Three”;
>
>In main program:
>  StaticConst.ONE = "One”;
>  StaticConst.TWO = "Two”;
>  StaticConst.THREE = "Three";
>
>  var /** @type {string} */ result1 = "One" + "Two" + "Three";
>  var /** @type {string} */ result2 = StaticConst.ONE + StaticConst.TWO +
>StaticConst.THREE;
>  var /** @type {string} */ result3 = Outside.ONE + Outside.TWO +
>Outside.THREE;
>  var /** @type {string} */ duplicate1 = "One" + "Two" + "Three";
>  var /** @type {string} */ duplicate2 = StaticConst.ONE +
>StaticConst.TWO + StaticConst.THREE;
>  var /** @type {string} */ duplicate3 = Outside.ONE + Outside.TWO +
>Outside.THREE;
>
>There doesn’t seem to be any optimisation in any of the three methods for
>debug and in fact using strings vs static const looks to increases the
>memory use of the code as you have multiple instances of “one”, “two”,
>“three”.It may do some optimisation at runtime I guess. But at a casual
>glance hard coding duplicate strings seem worse off although I agree it
>doesn’t have the object property lookup cost. I can’t see that would be
>significant.
>
>For release we get all three get treated exactly same way as Greg pointed
>out. In fact it goes as far as to optimise them all to be the same
>variable and use that single variable rather than have 6 different
>variables.
>
>So I can’t really see any reason for hard coding this in the SDK as it
>seems you get exactly the same results in production and probably better
>off in debug.
>
>Thanks,
>Justin

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [FlexJS] hard coded event names

Justin Mclean
Administrator
Hi,

> I was under the assumption that every constant adds an alias to the output.

That doesn’t seem to be the case as the compiler can optimise this - that’s a good think. Leet like get to make a cake and eat it.

> But again, I'd prefer that we don't spend time on this topic until after the release.

This is not going to effect the release - but I don’t see any reason to not talk about it / defer talking about it - scratch your own itch and all that.

Things can still happen when a release is being made if people don’t have the bandwidth to read every message they can be selective in what they read on this list or catch up later as probably was the case for most people at ApacheCon last week. No one there said sorry but can you not talk about the release because we’re busy on other stuff  :-)

Thanks,
Justin
Loading...