Skip to content

Commit

Permalink
Remove same-read-read and same-def-def Edges (#770)
Browse files Browse the repository at this point in the history
* feat: remove same-read-read and same-def-def edges

* lint-fix: handle inor linter problems
  • Loading branch information
EagleoutIce authored May 6, 2024
1 parent 5e05edc commit 7f30a7b
Show file tree
Hide file tree
Showing 17 changed files with 13 additions and 188 deletions.
6 changes: 0 additions & 6 deletions src/dataflow/graph/edge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ export const enum EdgeType {
Reads = 'reads',
/** The edge determines that source is defined by target */
DefinedBy = 'defined-by',
/** The edge determines that both nodes reference the same variable in a lexical/scoping sense, source and target are interchangeable (reads for at construction unbound variables) */
SameReadRead = 'same-read-read',
/** Similar to `same-read-read` but for def-def constructs without a read in-between */
SameDefDef = 'same-def-def',
/** The edge determines that the source calls the target */
Calls = 'calls',
/** The source returns target on call */
Expand Down Expand Up @@ -58,8 +54,6 @@ const traverseEdge: Record<EdgeType, TraverseEdge> = {
[EdgeType.DefinedByOnCall]: TraverseEdge.DefinedByOnCall,
[EdgeType.SideEffectOnCall]: TraverseEdge.SideEffect,
[EdgeType.NonStandardEvaluation]: TraverseEdge.Never,
[EdgeType.SameReadRead]: TraverseEdge.Never,
[EdgeType.SameDefDef]: TraverseEdge.Never,
[EdgeType.Returns]: TraverseEdge.Never
} as const

Expand Down
12 changes: 1 addition & 11 deletions src/dataflow/graph/graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,17 +258,7 @@ export class DataflowGraph<Vertex extends DataflowGraphVertexInfo = DataflowGrap
}

private installEdge(type: EdgeType, toId: NodeId, fromId: NodeId, edge: Edge) {
// sort (on id so that sorting is the same, independent of the attribute)
const bidirectional = type === EdgeType.SameReadRead || type === EdgeType.SameDefDef

if(bidirectional) {
const existingTo = this.edgeInformation.get(toId)
if(existingTo === undefined) {
this.edgeInformation.set(toId, new Map([[fromId, edge]]))
} else {
existingTo.set(fromId, edge)
}
} else if(type === EdgeType.DefinesOnCall) {
if(type === EdgeType.DefinesOnCall) {
const otherEdge: Edge = {
...edge,
types: new Set([EdgeType.DefinedByOnCall])
Expand Down
18 changes: 0 additions & 18 deletions src/dataflow/internal/linker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import type {
DataflowGraphVertexInfo,
FunctionArgument } from '../graph'
import {
CONSTANT_NAME,
isNamedArgument,
VertexType
} from '../graph'
Expand All @@ -18,11 +17,6 @@ import { EmptyArgument, RType } from '../../r-bridge'
import { slicerLogger } from '../../slicing'
import { dataflowLogger, EdgeType } from '../index'

export function linkIngoingVariablesInSameScope(graph: DataflowGraph, references: IdentifierReference[]): void {
const nameIdShares = produceNameSharedIdMap(references)
linkReadVariablesInSameScopeWithNames(graph, nameIdShares)
}

export type NameIdMap = DefaultMap<string, IdentifierReference[]>

export function produceNameSharedIdMap(references: IdentifierReference[]): NameIdMap {
Expand All @@ -35,18 +29,6 @@ export function produceNameSharedIdMap(references: IdentifierReference[]): NameI
return nameIdShares
}

export function linkReadVariablesInSameScopeWithNames(graph: DataflowGraph, nameIdShares: DefaultMap<string, IdentifierReference[]>) {
for(const [name, ids] of nameIdShares.entries()) {
if(ids.length <= 1 || name === CONSTANT_NAME) {
continue
}
const base = ids[0]
for(let i = 1; i < ids.length; i++) {
graph.addEdge(base.nodeId, ids[i].nodeId, { type: EdgeType.SameReadRead })
}
}
}

export function linkArgumentsOnCall(args: FunctionArgument[], params: RParameter<ParentInformation>[], graph: DataflowGraph): void {
const nameArgMap = new Map<string, IdentifierReference>(args.filter(isNamedArgument).map(a => [a.name, a] as const))
const nameParamMap = new Map<string, RParameter<ParentInformation>>(params.map(p => [p.name.content, p]))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ import type { DataflowProcessorInformation } from '../../../../../processor'
import { processDataflowFor } from '../../../../../processor'
import type { IdentifierReference, IEnvironment, REnvironmentInformation } from '../../../../../environments'
import { BuiltIn , makeAllMaybe, overwriteEnvironment, popLocalEnvironment, resolveByName } from '../../../../../environments'
import { linkFunctionCalls, linkReadVariablesInSameScopeWithNames } from '../../../../linker'
import { DefaultMap } from '../../../../../../util/defaultmap'
import { linkFunctionCalls } from '../../../../linker'
import type { DataflowGraphVertexInfo } from '../../../../../graph'
import { CONSTANT_NAME, DataflowGraph } from '../../../../../graph'
import { dataflowLogger, EdgeType } from '../../../../../index'
Expand Down Expand Up @@ -54,22 +53,10 @@ function processNextExpression(
remainingRead: Map<string, IdentifierReference[]>,
nextGraph: DataflowGraph
) {
// all inputs that have not been written until know, are read!
// all inputs that have not been written until now, are read!
for(const read of [...currentElement.in, ...currentElement.unknownReferences]) {
linkReadNameToWriteIfPossible(read, environment, listEnvironments, remainingRead, nextGraph)
}
// add same variable reads for deferred if they are read previously but not dependent
for(const writeTarget of currentElement.out) {
const writeName = writeTarget.name

const resolved = writeName ? resolveByName(writeName, environment) : undefined
if(resolved !== undefined) {
// write-write
for(const target of resolved) {
nextGraph.addEdge(target, writeTarget, { type: EdgeType.SameDefDef })
}
}
}
}

function updateSideEffectsForCalledFunctions(calledEnvs: {
Expand Down Expand Up @@ -174,11 +161,6 @@ export function processExpressionList<OtherInfo>(
}
}

if(expressions.length > 0) {
// now, we have to link same reads
linkReadVariablesInSameScopeWithNames(nextGraph, new DefaultMap(() => [], remainingRead))
}

dataflowLogger.trace(`expression list exits with ${remainingRead.size} remaining read names`)

if(defaultReturnExpr) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
} from '../../../../../environments'
import {
linkCircularRedefinitionsWithinALoop,
linkIngoingVariablesInSameScope,
produceNameSharedIdMap
} from '../../../../linker'
import { EdgeType } from '../../../../../graph'
Expand Down Expand Up @@ -91,7 +90,6 @@ export function processForLoop<OtherInfo>(

const outgoing = [...variable.out, ...writtenVariable, ...makeAllMaybe(body.out, nextGraph, outEnvironment, true)]

linkIngoingVariablesInSameScope(nextGraph, ingoing)
linkCircularRedefinitionsWithinALoop(nextGraph, nameIdShares, body.out)

patchFunctionCall({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,19 +73,6 @@ export function processFunctionDefinition<OtherInfo>(

dataflowLogger.trace(`Function definition with id ${name.info.id} has ${remainingRead.length} remaining reads`)

// link same-def-def with arguments
for(const writeTarget of body.out) {
const writeName = writeTarget.name

const resolved = writeName ? resolveByName(writeName, paramsEnvironments) : undefined
if(resolved !== undefined) {
// write-write
for(const target of resolved) {
subgraph.addEdge(target, writeTarget, { type: EdgeType.SameDefDef })
}
}
}

const outEnvironment = overwriteEnvironment(paramsEnvironments, bodyEnvironment)

for(const read of remainingRead) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
} from '../../../../../environments'
import { dataflowLogger, EdgeType } from '../../../../../index'
import { processKnownFunctionCall } from '../known-call-handling'
import { linkIngoingVariablesInSameScope } from '../../../../linker'
import { patchFunctionCall } from '../common'
import { unpackArgument } from '../argument/unpack-argument'

Expand Down Expand Up @@ -95,7 +94,6 @@ export function processIfThenElse<OtherInfo>(
...(makeThenMaybe ? makeAllMaybe(then?.out, nextGraph, finalEnvironment, true, rootId) : then?.out ?? []),
...(makeOtherwiseMaybe ? makeAllMaybe(otherwise?.out, nextGraph, finalEnvironment, true, rootId) : otherwise?.out ?? []),
]
linkIngoingVariablesInSameScope(nextGraph, ingoing)

patchFunctionCall({
nextGraph,
Expand Down
22 changes: 8 additions & 14 deletions src/util/mermaid/dfg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@ import type {
FunctionArgument,
IdentifierDefinition,
IdentifierReference,
IEnvironment } from '../../dataflow'
import { isNamedArgument
, isPositionalArgument
, VertexType,
IEnvironment,
EdgeType
} from '../../dataflow'
import { isNamedArgument,
isPositionalArgument,
VertexType,
BuiltIn,
BuiltInEnvironment,
CONSTANT_NAME,
EdgeType
CONSTANT_NAME
} from '../../dataflow'
import { guard } from '../assert'
import { escapeMarkdown, mermaidCodeToUrl } from './mermaid'
Expand Down Expand Up @@ -116,12 +117,6 @@ function displayFunctionArgMapping(argMapping: readonly FunctionArgument[]): str
return result.length === 0 ? '' : `\n (${result.join(', ')})`
}
function encodeEdge(from: string, to: string, types: Set<EdgeType | 'CD'>): string {
// sort from and to for same edges and relates be order independent
if(types.has(EdgeType.SameReadRead) || types.has(EdgeType.SameDefDef)) {
if(from > to) {
({ from, to } = { from: to, to: from })
}
}
return `${from}->${to}["${[...types].join(':')}"]`
}

Expand Down Expand Up @@ -194,11 +189,10 @@ function vertexToMermaid(info: DataflowGraphVertexInfo, mermaid: MermaidGraph, i
guard(edges !== undefined, `node ${id} must be found`)
const artificialCdEdges = (info.controlDependencies ?? []).map(x => [x, { types: new Set<EdgeType | 'CD'>(['CD']) }] as const)
for(const [target, edge] of [...edges[1], ...artificialCdEdges]) {
const dotEdge = edge.types.has(EdgeType.SameDefDef) || edge.types.has(EdgeType.SameReadRead)
const edgeId = encodeEdge(idPrefix + id, idPrefix + target, edge.types)
if(!mermaid.presentEdges.has(edgeId)) {
mermaid.presentEdges.add(edgeId)
mermaid.edgeLines.push(` ${idPrefix}${id} ${dotEdge ? '-.-' : '-->'}|"${[...edge.types].join(', ')}"| ${idPrefix}${target}`)
mermaid.edgeLines.push(` ${idPrefix}${id} -->|"${[...edge.types].join(', ')}"| ${idPrefix}${target}`)
if(mermaid.mark?.has(id + '->' + target)) {
// who invented this syntax?!
mermaid.edgeLines.push(` linkStyle ${mermaid.presentEdges.size - 1} stroke:red,color:red,stroke-width:4px;`)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ export function printAsBuilder(graph: DataflowGraph): string {
const EdgeTypeFnMap: Record<EdgeType, string | undefined> = {
[EdgeType.Reads]: 'reads',
[EdgeType.DefinedBy]: 'definedBy',
[EdgeType.SameReadRead]: 'sameRead',
[EdgeType.SameDefDef]: 'sameDef',
[EdgeType.Calls]: 'calls',
[EdgeType.Returns]: 'returns',
[EdgeType.DefinesOnCall]: 'definesOnCall',
Expand Down Expand Up @@ -312,9 +310,5 @@ class DataflowBuilderPrinter {
}

function edgeId(from: NodeId, to: NodeId, type: EdgeType): string {
if(type === EdgeType.SameReadRead || type === EdgeType.SameDefDef) {
// we don't care about the direction
[from, to] = from > to ? [to, from] : [from, to]
}
return `${from}->${to}[${type}]`
}
20 changes: 0 additions & 20 deletions test/functionality/_helper/dataflow/dataflowgraph-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,26 +220,6 @@ export class DataflowGraphBuilder extends DataflowGraph {
return this.edgeHelper(from, to, EdgeType.DefinedBy)
}

/**
* Adds a **same-read-read edge** (E3), with from and to as two variable uses
* on the same variable.
*
* @see reads for parameters.
*/
public sameRead(from: NodeId, to: DataflowGraphEdgeTarget) {
return this.edgeHelper(from, to, EdgeType.SameReadRead)
}

/**
* Adds a **same-def-def edge** (E4), with from and to as two variables
* that share a defining variable.
*
* @see reads for parameters.
*/
public sameDef(from: NodeId, to: DataflowGraphEdgeTarget) {
return this.edgeHelper(from, to, EdgeType.SameDefDef)
}

/**
* Adds a **call edge** (E5) with from as caller, and to as callee.
*
Expand Down
Loading

2 comments on commit 7f30a7b

@github-actions
Copy link

Choose a reason for hiding this comment

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

"artificial" Benchmark Suite

Benchmark suite Current: 7f30a7b Previous: 1e5ddeb Ratio
Retrieve AST from R code 296.0169118181818 ms 305.50355490909095 ms 0.97
Normalize R AST 32.42537236363636 ms 33.04192136363636 ms 0.98
Produce dataflow information 66.65919663636363 ms 66.58585145454545 ms 1.00
Total per-file 1508.7309028636362 ms 1512.547620909091 ms 1.00
Static slicing 1.4012763801984116 ms (1.2953921382764266) 1.3817132831241319 ms (1.27303105869458) 1.01
Reconstruct code 0.4543032771693104 ms (0.24422779283891366) 0.45135141022522257 ms (0.25927254974160097) 1.01
Total per-slice 1.8710348922657893 ms (1.3502084612283285) 1.8475447304133532 ms (1.3326721674469058) 1.01
failed to reconstruct/re-parse 0 # 0 # 1
times hit threshold 0 # 0 # 1
reduction (characters) 0.7329390759026897 # 0.7329390759026897 # 1
reduction (normalized tokens) 0.7209834969577295 # 0.7209834969577295 # 1

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link

Choose a reason for hiding this comment

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

"social-science" Benchmark Suite

Benchmark suite Current: 7f30a7b Previous: 1e5ddeb Ratio
Retrieve AST from R code 319.82132466 ms 330.55613118 ms 0.97
Normalize R AST 35.74594172 ms 36.10837608 ms 0.99
Produce dataflow information 187.37990026 ms 188.6117503 ms 0.99
Total per-file 3747.75382854 ms 3858.78617088 ms 0.97
Static slicing 8.598805396820248 ms (14.999793798482417) 8.763488287471356 ms (15.142795018273485) 0.98
Reconstruct code 0.5235691192413336 ms (0.2986719559125563) 0.6295735167403027 ms (0.28826473087031307) 0.83
Total per-slice 9.131787729106012 ms (15.150035645807264) 9.403718790030862 ms (15.25727767695505) 0.97
failed to reconstruct/re-parse 7 # 7 # 1
times hit threshold 298 # 298 # 1
reduction (characters) 0.8935817303062389 # 0.8935817303062389 # 1
reduction (normalized tokens) 0.8531248144961375 # 0.8531248144961375 # 1

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.