Re: git commit: [flex-sdk] [refs/heads/develop] - fix up new MXML codegen and Databinding codegen handling

[Justin Mclean <ju...@classsoftware.com>]

Hi,

Can you give a little more detail about this change and why you made it? What exactly is the "new" MXML codegen? How does performance compare before and after these changes?

Updating the Release notes would also be nice.

Thanks,
Justin

Re: git commit: [flex-sdk] [refs/heads/develop] - fix up new MXML codegen and Databinding codegen handling

[Alex Harui <ah...@adobe.com>]

Well, the changes you copied into this thread I think are all in Falcon
java files aren't they?  The code checked into the SDK should be gated by
that one null check and some generated code from Falcon.

And the net is that Falcon generated the new codegen and BasicTests
passed.  I'm now debugging my internal customer's huge app.  I think the
SDK changes have been in long enough for Mustella to pass as well.

I'll try doing more local commits, but sometimes with Falcon code when I
tried that I ended up reverting some commits and that also made the check
in hard to read.

-Alex

On 11/2/13 8:38 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:

>Hi,
>
>> Yeah, sorry.  This is all for Falcon.  Doesn't affect MXMLC SWFs at all.
>
>Are you 100% sure? I'm not sure and not looked at the code closely but it
>looks like more than a null check to me.
>
>Looks like there quite a few changes in there and there a couple of
>things that look a little odd at first viewing. I'm not familiar with the
>code so it hard to tell what all these changes are actually for.
>
>For instance logic here you have && ! flex SDK and then else || FlexSDK
>shouldn't the second be an &&? Again I don't know and it could be fine it
>just looks odd on first viewing.
>+            if (s.contains(".") && !isFlexSDK)
>            {
>                String[] parts = s.split("\\.");
>                for (String part : parts)
>                    ret.addInstruction(OP_pushstring, part);
>                ret.addInstruction(OP_newarray, parts.length);
>            }
>-            else if (s == null || s.length() == 0)
>+            else if (s == null || s.length() == 0 || isFlexSDK
>
>And in one part this line was removed "for (int i = 0; i <
>node.getChildCount(); i++)" and replaced with something else, but a
>little further down it's left in.
>
>And changes like this just seem mysterious.
>-            propertyCount += 4;
>+            propertyCount += 5;
>
>I would of expected changes like large to be a merge of a branch with at
>least a dozen check in comments. Done like this it makes it quite hard to
>review. Might also be helpful to some discussion on the list about the
>changes and/or some comments in the code.
>
>Thanks,
>Justin

Re: git commit: [flex-sdk] [refs/heads/develop] - fix up new MXML codegen and Databinding codegen handling

[Justin Mclean <ju...@classsoftware.com>]

Hi,

> Yeah, sorry.  This is all for Falcon.  Doesn't affect MXMLC SWFs at all.

Are you 100% sure? I'm not sure and not looked at the code closely but it looks like more than a null check to me.

Looks like there quite a few changes in there and there a couple of things that look a little odd at first viewing. I'm not familiar with the code so it hard to tell what all these changes are actually for.

For instance logic here you have && ! flex SDK and then else || FlexSDK shouldn't the second be an &&? Again I don't know and it could be fine it just looks odd on first viewing.
+            if (s.contains(".") && !isFlexSDK)
            {
                String[] parts = s.split("\\.");
                for (String part : parts)
                    ret.addInstruction(OP_pushstring, part);
                ret.addInstruction(OP_newarray, parts.length);
            }
-            else if (s == null || s.length() == 0)
+            else if (s == null || s.length() == 0 || isFlexSDK

And in one part this line was removed "for (int i = 0; i < node.getChildCount(); i++)" and replaced with something else, but a little further down it's left in.

And changes like this just seem mysterious.
-            propertyCount += 4;
+            propertyCount += 5;

I would of expected changes like large to be a merge of a branch with at least a dozen check in comments. Done like this it makes it quite hard to review. Might also be helpful to some discussion on the list about the changes and/or some comments in the code.

Thanks,
Justin

Re: git commit: [flex-sdk] [refs/heads/develop] - fix up new MXML codegen and Databinding codegen handling

[Alex Harui <ah...@adobe.com>]

Yeah, sorry.  This is all for Falcon.  Doesn't affect MXMLC SWFs at all.
The initial code paths went in in 4.10 I think.   Unless I made a mistake
there should only be one null check per UIComponent instance that MXMLC
SWFs will see.

-Alex

On 11/2/13 4:09 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:

>Hi,
>
>Can you give a little more detail about this change and why you made it?
>What exactly is the "new" MXML codegen? How does performance compare
>before and after these changes?
>
>Updating the Release notes would also be nice.
>
>Thanks,
>Justin