Skip to content

Conversation

@aschackmull
Copy link
Contributor

This improves the call contexts used in data flow by also considering the type of the instance argument. Recent experience from C# has shown this to be especially helpful when dealing with the visitor pattern.

@aschackmull aschackmull requested a review from a team as a code owner August 14, 2020 13:55
@hvitved
Copy link
Contributor

hvitved commented Aug 14, 2020

Curious to see what the effect is on performance and results.

@adityasharad adityasharad changed the base branch from master to main August 14, 2020 18:33
@aschackmull
Copy link
Contributor Author

@aschackmull
Copy link
Contributor Author

With a suitable test configuration in scope, quick-eval'ing the following on JDK11 yields:

    pn = count(PathNode pn_) and
    pnr = count(PathNode pn_ | reach(pn_))
pn pnr
Before 315048 238285
After 306711 206954

@aschackmull aschackmull force-pushed the java/dispatch-ctx-this-param branch from 312af37 to bcad18f Compare August 20, 2020 13:36
@aschackmull
Copy link
Contributor Author

I've rebased and rerun the tuple counts since we now apply call contexts in the final pruning stage. Numbers are still looking good:

fwd4 nc4 fwd4t nc4t pn pnr
Before 113281 108944 403062 322831 313277 238199
After 108602 104193 392612 305260 303797 206388

@aschackmull
Copy link
Contributor Author

Copy link
Contributor

@aibaars aibaars left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@codeql-ci codeql-ci merged commit 311e62f into github:main Sep 1, 2020
@aschackmull aschackmull deleted the java/dispatch-ctx-this-param branch September 1, 2020 14:06
@hvitved
Copy link
Contributor

hvitved commented Sep 1, 2020

Seems like this had real impact 🎉 :

java/tainted-arithmetic                                                     +0 -99
java/uncontrolled-arithmetic                                                +0 -113

aschackmull added a commit to aschackmull/ql that referenced this pull request Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants