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.

Sign up for GitHub

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

Temporary fix to speed up canAvoidInstr #1699

Merged
merged 14 commits into from
Apr 29, 2024
Merged

Temporary fix to speed up canAvoidInstr #1699

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
da818e4
check if env instruction is necessary
DiligentPenguinn Apr 22, 2024
de09673
Add test cases
DiligentPenguinn Apr 22, 2024
4d26880
Implement helper functions
DiligentPenguinn Apr 22, 2024
067bc95
Fix format
DiligentPenguinn Apr 22, 2024
95c5e7d
Cache isEnvDependent results
DiligentPenguinn Apr 27, 2024
432b1a8
Delete unnecessary comments
DiligentPenguinn Apr 27, 2024
0e0f5db
Merge branch 'master' into cse-machine-simplify-env-instr
DiligentPenguinn Apr 27, 2024
04917eb
Merge branch 'master' into cse-machine-simplify-env-instr
DiligentPenguinn Apr 27, 2024
5504278
Merge branch 'master' into cse-machine-simplify-env-instr
DiligentPenguinn Apr 27, 2024
5e2f015
Speed up env dependent checking, approach 2
DiligentPenguinn Apr 27, 2024
1dd9845
Merge branch 'cse-machine-simplify-env-instr' of https://github.com/s…
DiligentPenguinn Apr 27, 2024
b88ad72
merge
DiligentPenguinn Apr 27, 2024
eca7bbf
Address my comments
RichDom2185 Apr 29, 2024
350ea87
Merge branch 'master' into cse-machine-simplify-env-instr
RichDom2185 Apr 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/cse-machine/__tests__/__snapshots__/cse-machine-heap.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ EnvTree {
"type": "Identifier",
},
"end": 101,
"isEnvDependent": true,
"loc": SourceLocation {
"end": Position {
"column": 12,
Expand Down Expand Up @@ -798,6 +799,7 @@ EnvTree {
"type": "Identifier",
},
"end": 101,
"isEnvDependent": true,
"loc": SourceLocation {
"end": Position {
"column": 12,
Expand Down Expand Up @@ -1443,6 +1445,7 @@ EnvTree {
"type": "Identifier",
},
"end": 101,
"isEnvDependent": true,
"loc": SourceLocation {
"end": Position {
"column": 12,
Expand Down Expand Up @@ -2109,6 +2112,7 @@ EnvTree {
"type": "Identifier",
},
"end": 101,
"isEnvDependent": true,
"loc": SourceLocation {
"end": Position {
"column": 12,
Expand Down Expand Up @@ -2500,6 +2504,7 @@ EnvTree {
"type": "Identifier",
},
"end": 101,
"isEnvDependent": true,
"loc": SourceLocation {
"end": Position {
"column": 12,
Expand Down Expand Up @@ -2678,6 +2683,7 @@ EnvTree {
"type": "Identifier",
},
"end": 101,
"isEnvDependent": true,
"loc": SourceLocation {
"end": Position {
"column": 12,
Expand Down
22 changes: 22 additions & 0 deletions src/cse-machine/__tests__/__snapshots__/cse-machine-runtime-context.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ Control {
"storage": Array [
Object {
"instrType": "BinaryOperation",
"isEnvDependent": false,
"srcNode": Node {
"end": 53,
"left": Node {
Expand Down Expand Up @@ -102,6 +103,7 @@ Control {
"operator": "*",
"right": Node {
"end": 53,
"isEnvDependent": false,
"loc": SourceLocation {
"end": Position {
"column": 14,
Expand All @@ -124,6 +126,7 @@ Control {
},
Node {
"end": 53,
"isEnvDependent": false,
"loc": SourceLocation {
"end": Position {
"column": 14,
Expand All @@ -141,6 +144,7 @@ Control {
},
Object {
"instrType": "BinaryOperation",
"isEnvDependent": false,
"srcNode": Node {
"end": 53,
"left": Node {
Expand Down Expand Up @@ -238,6 +242,7 @@ Control {
"operator": "*",
"right": Node {
"end": 53,
"isEnvDependent": false,
"loc": SourceLocation {
"end": Position {
"column": 14,
Expand All @@ -260,6 +265,7 @@ Control {
},
Node {
"end": 53,
"isEnvDependent": false,
"loc": SourceLocation {
"end": Position {
"column": 14,
Expand All @@ -277,6 +283,7 @@ Control {
},
Object {
"instrType": "BinaryOperation",
"isEnvDependent": false,
"srcNode": Node {
"end": 53,
"left": Node {
Expand Down Expand Up @@ -374,6 +381,7 @@ Control {
"operator": "*",
"right": Node {
"end": 53,
"isEnvDependent": false,
"loc": SourceLocation {
"end": Position {
"column": 14,
Expand Down Expand Up @@ -403,6 +411,7 @@ Control {
"storage": Array [
Object {
"instrType": "BinaryOperation",
"isEnvDependent": false,
"srcNode": Node {
"end": 53,
"left": Node {
Expand Down Expand Up @@ -521,6 +530,7 @@ Control {
},
Object {
"instrType": "BinaryOperation",
"isEnvDependent": false,
"srcNode": Node {
"end": 53,
"left": Node {
Expand Down Expand Up @@ -639,6 +649,7 @@ Control {
},
Object {
"instrType": "BinaryOperation",
"isEnvDependent": false,
"srcNode": Node {
"end": 53,
"left": Node {
Expand Down Expand Up @@ -766,6 +777,7 @@ Control {
"end": 86,
"expression": Node {
"end": 85,
"isEnvDependent": true,
"left": Node {
"end": 81,
"loc": SourceLocation {
Expand Down Expand Up @@ -813,6 +825,7 @@ Control {
"start": 80,
"type": "AssignmentExpression",
},
"isEnvDependent": true,
"loc": SourceLocation {
"end": Position {
"column": 6,
Expand All @@ -832,6 +845,7 @@ Control {
"end": 86,
"expression": Node {
"end": 85,
"isEnvDependent": true,
"left": Node {
"end": 81,
"loc": SourceLocation {
Expand Down Expand Up @@ -879,6 +893,7 @@ Control {
"start": 80,
"type": "AssignmentExpression",
},
"isEnvDependent": true,
"loc": SourceLocation {
"end": Position {
"column": 6,
Expand Down Expand Up @@ -2541,8 +2556,10 @@ Control {
"end": 43,
"expression": Node {
"end": 42,
"isEnvDependent": false,
"left": Node {
"end": 38,
"isEnvDependent": false,
"loc": SourceLocation {
"end": Position {
"column": 5,
Expand Down Expand Up @@ -2571,6 +2588,7 @@ Control {
"operator": "+",
"right": Node {
"end": 42,
"isEnvDependent": false,
"loc": SourceLocation {
"end": Position {
"column": 9,
Expand All @@ -2589,6 +2607,7 @@ Control {
"start": 37,
"type": "BinaryExpression",
},
"isEnvDependent": false,
"loc": SourceLocation {
"end": Position {
"column": 10,
Expand All @@ -2606,6 +2625,7 @@ Control {
"end": 50,
"expression": Node {
"end": 49,
"isEnvDependent": false,
"loc": SourceLocation {
"end": Position {
"column": 5,
Expand All @@ -2621,6 +2641,7 @@ Control {
"type": "Literal",
"value": 3,
},
"isEnvDependent": false,
"loc": SourceLocation {
"end": Position {
"column": 6,
Expand All @@ -2635,6 +2656,7 @@ Control {
"type": "ExpressionStatement",
},
],
"isEnvDependent": false,
"loc": SourceLocation {
"end": Position {
"column": 1,
Expand Down
4 changes: 3 additions & 1 deletion src/cse-machine/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ export type Instr =
| GenContInstr
| ResumeContInstr

export type ControlItem = Node | Instr
export type ControlItem = (Node | Instr) & {
isEnvDependent?: boolean
}

// Every array also has the properties `id` and `environment` for use in the frontend CSE Machine
export type EnvArray = any[] & {
Expand Down
40 changes: 23 additions & 17 deletions src/cse-machine/utils.ts
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with this as long as it is a temporary hotfix. Please create an issue to migrate this to the node creator in the future.

Thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #1702.

Original file line number Diff line number Diff line change
Expand Up @@ -672,9 +672,16 @@ export const hasContinueStatement = (block: es.BlockStatement | StatementSequenc
* control in SIMPLE CASES, so it might not be exhaustive
*/
export const isEnvDependent = (command: ControlItem): boolean => {
// If the result is already calculated, return it
if (command.isEnvDependent != undefined) {
return command.isEnvDependent
}

// Otherwise, calculate and store the result
let isDependent = true
if (isInstr(command)) {
const type = command.instrType
return !(
isDependent = !(
type === InstrType.UNARY_OP ||
type === InstrType.BINARY_OP ||
type === InstrType.POP ||
Expand All @@ -688,25 +695,28 @@ export const isEnvDependent = (command: ControlItem): boolean => {
const type = command.type
switch (type) {
case 'StatementSequence':
let isDependent = false
command.body.forEach(function (statement: es.Statement) {
isDependent = isEnvDependent(statement) || isDependent
})
return isDependent
isDependent = command.body.some((statement: es.Statement) => isEnvDependent(statement))
case 'Literal':
return false
isDependent = false
break
case 'BinaryExpression':
return isEnvDependent(command.left) || isEnvDependent(command.right)
isDependent = isEnvDependent(command.left) || isEnvDependent(command.right)
break
case 'LogicalExpression':
return isEnvDependent(command.left) || isEnvDependent(command.right)
isDependent = isEnvDependent(command.left) || isEnvDependent(command.right)
break
case 'UnaryExpression':
return isEnvDependent(command.argument)
isDependent = isEnvDependent(command.argument)
break
case 'ExpressionStatement':
return isEnvDependent(command.expression)
isDependent = isEnvDependent(command.expression)
break
default:
return true
break
}
}
command.isEnvDependent = isDependent
return isDependent
}

/**
Expand All @@ -717,9 +727,5 @@ export const isEnvDependent = (command: ControlItem): boolean => {
* control in SIMPLE CASES, so it might not be exhaustive
*/
export const canAvoidEnvInstr = (control: Control): boolean => {
let canAvoid = true
control.getStack().forEach(function (command: ControlItem) {
canAvoid = canAvoid && !isEnvDependent(command)
})
return canAvoid
return !control.getStack().some((command: ControlItem) => isEnvDependent(command))
}
1 change: 0 additions & 1 deletion src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,6 @@ export type Node =
| StatementSequence
| es.MaybeNamedClassDeclaration
| es.MaybeNamedFunctionDeclaration

/*
Although the ESTree specifications supposedly provide a Directive interface, the index file does not seem to export it.
As such this interface was created here to fulfil the same purpose.
Expand Down
Loading