Skip to content

JavaScript: Tweak a few predicates in the data flow library.#406

Merged
semmle-qlci merged 3 commits into
github:masterfrom
xiemaisi:js/configuration-fiddling
Nov 6, 2018
Merged

JavaScript: Tweak a few predicates in the data flow library.#406
semmle-qlci merged 3 commits into
github:masterfrom
xiemaisi:js/configuration-fiddling

Conversation

@xiemaisi
Copy link
Copy Markdown

@xiemaisi xiemaisi commented Nov 5, 2018

During a recent customer visit, we noticed that a pragma[noopt] introduced by @asger-semmle in 0936cda was forcing a very bad join order, so I removed it (first commit). I also introduced two annotations to avoid unhelpful magic (second commit and third commit), and extracted an auxiliary predicate to enable a better join order (third commit).

On default.slugs, these three changes together have no impact on performance or results, but they do improve performance quite a bit on said customer's code base, and I'm always happy when I can remove a noopt.

@asger-semmle, do you remember where you observed the slow join that caused you to introduce the pragma in the first place? Was it one of our default projects?

@xiemaisi xiemaisi added the JS label Nov 5, 2018
@xiemaisi xiemaisi requested a review from a team as a code owner November 5, 2018 17:01
@asger-semmle
Copy link
Copy Markdown
Contributor

It's not fresh in memory obviously but I still have the some log files from that line of work. Based on the date of the commit, I believe it fixed the last three projects in this run.

@xiemaisi
Copy link
Copy Markdown
Author

xiemaisi commented Nov 5, 2018

OK, those are all in the default suite, so something else (or one of the other two commits in this PR) must have fixed them even without the noopt.

Copy link
Copy Markdown
Contributor

@asger-semmle asger-semmle left a comment

Choose a reason for hiding this comment

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

LGTM

@semmle-qlci semmle-qlci merged commit 76475fe into github:master Nov 6, 2018
@xiemaisi xiemaisi deleted the js/configuration-fiddling branch November 6, 2018 14:25
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.

3 participants