Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Task]: Generate new typedesc instruction for record and tuple when type descriptor resolving #38844

Open
chiranSachintha opened this issue Nov 28, 2022 · 4 comments · Fixed by #43588 · May be fixed by #43596
Open

[Task]: Generate new typedesc instruction for record and tuple when type descriptor resolving #38844

chiranSachintha opened this issue Nov 28, 2022 · 4 comments · Fixed by #43588 · May be fixed by #43596
Assignees
Labels
Team/CompilerFE All issues related to Language implementation and Compiler, this exclude run times. Type/Task userCategory/Compilation
Milestone

Comments

@chiranSachintha
Copy link
Member

Description

In the current implementation, new typedesc instruction generates when value is created.
When considering the following example. we create two new typedesc instructions for record{int k = 10;} type.

function foo() {
    record{int k = 10;} m;
    m = {};
    m = {k: 15};
}

Describe your task(s)

No response

Related area

-> Compilation

Related issue(s) (optional)

No response

Suggested label(s) (optional)

No response

Suggested assignee(s) (optional)

No response

@chiranSachintha chiranSachintha self-assigned this Nov 28, 2022
@chiranSachintha chiranSachintha added the Team/CompilerFE All issues related to Language implementation and Compiler, this exclude run times. label Nov 28, 2022
@chiranSachintha chiranSachintha added the Points/8 Equivalent to eight day effort label Nov 28, 2022
@ballerina-bot ballerina-bot added needTriage The issue has to be inspected and labeled manually userCategory/Compilation labels Nov 28, 2022
@chiranSachintha chiranSachintha added Points/3 Equivalent to three days effort and removed needTriage The issue has to be inspected and labeled manually Points/8 Equivalent to eight day effort labels Dec 2, 2022
@MaryamZi MaryamZi moved this from In Progress to BackLog in Ballerina Team Main Board Feb 2, 2023
@chiranSachintha chiranSachintha changed the title [Task]: Generate new typedesc instruction for record when typedesc resolving [Task]: Generate new typedesc instruction for record and tuple when type descriptor resolving Feb 9, 2023
@chiranSachintha chiranSachintha moved this from BackLog to In Progress in Ballerina Team Main Board Feb 22, 2023
@chiranSachintha chiranSachintha removed the Points/3 Equivalent to three days effort label Apr 17, 2023
@dulajdilshan dulajdilshan moved this from In Progress to Planned for Sprint in Ballerina Team Main Board Apr 27, 2023
@chiranSachintha chiranSachintha moved this from Planned for Sprint to In Progress in Ballerina Team Main Board May 9, 2023
@chiranSachintha chiranSachintha moved this from In Progress to Planned for Sprint in Ballerina Team Main Board Jun 15, 2023
@chiranSachintha chiranSachintha moved this from Planned for Sprint to In Progress in Ballerina Team Main Board Sep 5, 2023
@chiranSachintha chiranSachintha moved this from In Progress to PR Sent in Ballerina Team Main Board Jan 9, 2024
@chiranSachintha chiranSachintha moved this from PR Sent to In Progress in Ballerina Team Main Board Jan 17, 2024
@chiranSachintha chiranSachintha moved this from In Progress to PR Sent in Ballerina Team Main Board Jan 24, 2024
@dulajdilshan dulajdilshan moved this from PR Sent to Planned for Sprint in Ballerina Team Main Board Mar 13, 2024
@MaryamZi MaryamZi added this to the 2201.10.0 milestone Jun 25, 2024
@keizer619 keizer619 moved this from Planned for Sprint to BackLog in Ballerina Team Main Board Jun 27, 2024
@MaryamZi MaryamZi moved this from BackLog to On Hold in Ballerina Team Main Board Jul 3, 2024
@chiranSachintha chiranSachintha moved this from On Hold to Planned for Sprint in Ballerina Team Main Board Jul 3, 2024
@rdulmina
Copy link
Contributor

rdulmina commented Oct 18, 2024

Changes are done. 13 jballerina unit tests are filing ATM. Will send a PR soon. Below are some failing cases.

  1. The below sample fails with java.lang.NullPointerException: Cannot read field "name" because "varDcl" is null
public function main() {
    var input = [{name: "Saman", price: 11}];
    record {|string name; int price;|}[][] res = from var {name} in input group by name
                                                    select from var {price} in input
                                                            select {name, price};
// the issue here is we create several lambda functions for the query expression and those functions 
// do not have the typedesc in local variables. it seems typedesc need to be passed as a closure for those
}
  1. Set of test are failing causing method to large error.

@rdulmina
Copy link
Contributor

Large method issue is due to how we create typedesc statements from the Desugar. Consider the below sample.

public function main() {

[record {|string name; int age;|}, record {|string name; int age;|}, record {|string name; int age;|}] a = [{name: "apple", age: 1}, {name: "apple", age: 1}, {name: "apple", age: 1}];

}

Before BIR

(note the %5 is reused to create typedesc vars for all the three record types)

%1(LOCAL) ($anonType$_0, $anonType$_1, $anonType$_2);
%2(TEMP) typeDesc<any|error>;
%4(TEMP) int;
%5(TEMP) typeDesc<any|error>;
%6(TEMP) $anonType$_0;
%7(TEMP) string;
%8(TEMP) string;
%9(TEMP) string;
%10(TEMP) int;
%12(TEMP) $anonType$_1;
%18(TEMP) $anonType$_2;

bb0 {
%2 = newType ($anonType$_0, $anonType$_1, $anonType$_2);
%4 = ConstLoad 3;
%5 = newType $anonType$_0;
%7 = ConstLoad name;
%8 = ConstLoad apple;
%9 = ConstLoad age;
%10 = ConstLoad 1;
%6 = NewMap %5{%7:%8,%9:%10};
%5 = newType $anonType$_1;
%7 = ConstLoad name;
%8 = ConstLoad apple;
%9 = ConstLoad age;
%10 = ConstLoad 1;
%12 = NewMap %5{%7:%8,%9:%10};
%5 = newType $anonType$_2;
%7 = ConstLoad name;
%8 = ConstLoad apple;
%9 = ConstLoad age;
%10 = ConstLoad 1;
%18 = NewMap %5{%7:%8,%9:%10};
%1 = newArray %2[%4]{%6,%12,%18};

After BIR

With the typedesc creation from the Desugar phase we define the typedesc at the beginning of the block. Hence he variables cannot be reused. When there is very large tuple than the one in the sample, this will cause the method to large error

%1(SYNTHETIC) typeDesc<$anonType$_0>;
%3(SYNTHETIC) typeDesc<$anonType$_1>;
%5(SYNTHETIC) typeDesc<$anonType$_2>;

%7(SYNTHETIC) typeDesc<($anonType$_0, $anonType$_1, $anonType$_2)>;
%9(LOCAL) ($anonType$_0, $anonType$_1, $anonType$_2);
%11(TEMP) int;
%12(TEMP) $anonType$_0;
%13(TEMP) string;
%14(TEMP) string;
%15(TEMP) string;
%16(TEMP) int;
%17(TEMP) $anonType$_1;
%22(TEMP) $anonType$_2;

bb0 {

%1 = newType $anonType$_0;
%3 = newType $anonType$_1;
%5 = newType $anonType$_2;

%7 = newType ($anonType$_0, $anonType$_1, $anonType$_2);
%11 = ConstLoad 3;
%13 = ConstLoad name;
%14 = ConstLoad apple;
%15 = ConstLoad age;
%16 = ConstLoad 1;
%12 = NewMap %1{%13:%14,%15:%16};
%13 = ConstLoad name;
%14 = ConstLoad apple;
%15 = ConstLoad age;
%16 = ConstLoad 1;
%17 = NewMap %3{%13:%14,%15:%16};

@rdulmina
Copy link
Contributor

rdulmina commented Nov 1, 2024

Another reson that could cause large method error

Consider the below sample

public function main() {
	[record {||}, record {||}] a = [{}, {}];
	a =  [{}, {}];
}

We will generate local type desc statements for the above anonymous type. Consider the below BIR Dump.


1. public main function() -> () {
2.     %0(RETURN) ();
3.     %1(SYNTHETIC) typeDesc<($anonType$_0, $anonType$_1)>;
4.     %3(SYNTHETIC) typeDesc<$anonType$_0>;
5.     %5(SYNTHETIC) typeDesc<$anonType$_1>;
6.     %7(LOCAL) ($anonType$_0, $anonType$_1);
7.     %9(TEMP) int;
8.     %10(TEMP) $anonType$_0;
9.     %11(TEMP) $anonType$_1;
10. 
11.     bb0 {
12.         %1 = newType ($anonType$_0, $anonType$_1);
13.         %3 = newType $anonType$_0;
14.         %5 = newType $anonType$_1;
15.         %9 = ConstLoad 2;
16.         %10 = NewMap %3{};
17.         %11 = NewMap %5{};
18.         %7 = newArray %1[%9]{%10,%11};
19.         %9 = ConstLoad 2;
20.         %10 = NewMap %3{};
21.         %11 = NewMap %5{};
22.         %7 = newArray %1[%9]{%10,%11};
23.         %0 = ConstLoad 0;
24.         GOTO bb1;
25.     }
26.     bb1 {
27.         return;
28.     }
29. 
30. 
31. }

When there is a large method and we try to split it. We can Just take the L#12 - L#18 to a separate function sice when we are creating a new value from that type again, we will need the %1, %2, %3. Because of that we need to pass those as arguments to the split function. When there are more than 125 arguments we dont split the method.

So with this change there can be such cases where the split did not happen due to large number of arguments. Currently the limit is increased upto 220. Max is 225.

Previously this did not happen becasue we create duplicate typedesc instructions for the second array literal. This does not happen for types which has a type definition since the type desc var is global

@rdulmina
Copy link
Contributor

rdulmina commented Dec 2, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment