Skip to content

C++: Get rid of a fastTC and noopt in IR#2519

Merged
rdmarsh2 merged 1 commit into
github:masterfrom
jbj:ir-backedge-notc
Dec 18, 2019
Merged

C++: Get rid of a fastTC and noopt in IR#2519
rdmarsh2 merged 1 commit into
github:masterfrom
jbj:ir-backedge-notc

Conversation

@jbj

@jbj jbj commented Dec 12, 2019

Copy link
Copy Markdown
Contributor

The getAChild* fastTC was causing OOM on a make allyesconfig Linux database with 8GB RAM, and I've observed it to be slow on other databases too.

The `getAChild*` fastTC was causing OOM on a `make allyesconfig` Linux
database with 8GB RAM, and I've observed it to be slow on other
databases too.
@jbj jbj added the C++ label Dec 12, 2019
@jbj jbj requested a review from a team as a code owner December 12, 2019 08:37
@jbj

jbj commented Dec 12, 2019

Copy link
Copy Markdown
Contributor Author

By the way, the problematic TC was introduced by myself in #812.

@jbj jbj requested review from dbartol and rdmarsh2 December 12, 2019 14:38

@geoffw0 geoffw0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes LGTM. I've asked one question but it shouldn't be considered blocking.

I did experience a slight (7% / 2%) slowdown on the two projects I tested this with, but that's with just one IR query (RedundantNullCheckSimple.ql) and these numbers might not be significant. If you've done more extensive testing, I'm happy.

@geoffw0 geoffw0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@rdmarsh2 rdmarsh2 merged commit 33067c8 into github:master Dec 18, 2019
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