Skip to content

Conversation

criemen
Copy link
Collaborator

@criemen criemen commented Nov 18, 2020

Performance numbers:
Using 16 threads for evaluator, with empty cache
boost, lockpool.cpp
before: total 85s, qlClass 10s, EB 30s
after: total 75s, qlClass 3s

openjdk, ad_x86.cpp
before: total 93s, qlClass 11s, EB 28
after: total 81s, qlClass 7.9s

The times for the individual predicates are from the query log, the total time is the wall time from the parallel execution.
EB is the predicate in ElementBase that computes the el that need the default implementation of getAPrimaryQlClass, i.e. where the predicate is not overriden.
Note: Changing this default from "???" to no result is a breaking API change.

Part of https://github.com/github/codeql-c-analysis-team/issues/129

@criemen criemen requested a review from a team as a code owner November 18, 2020 15:11
@github-actions github-actions bot added the C++ label Nov 18, 2020
@criemen criemen marked this pull request as draft November 18, 2020 15:21
@criemen criemen marked this pull request as ready for review November 18, 2020 16:48
@criemen
Copy link
Collaborator Author

criemen commented Nov 18, 2020

New performance numbers, see first post.
Unfortunately the first version pruned a bit too much, and was even faster, but incorrect.

Copy link
Contributor

@jbj jbj left a comment

Choose a reason for hiding this comment

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

Something's a bit dodgy with the performance numbers you posted:

boost, lockpool.cpp
before: total 85s, qlClass 10s, EB 30s
after: total 75s, qlClass 3s

37 seconds are removed from the predicates you're tracking, but only 10 seconds are removed from the total. So there must be other predicates that are worth tracking.

@criemen
Copy link
Collaborator Author

criemen commented Nov 19, 2020

The performance numbers are quite weird tbh - maybe I am using too high parallelism to really see the effects of the optimization here.
We can discuss this in our call tomorrow.

@criemen
Copy link
Collaborator Author

criemen commented Nov 20, 2020

I have now new performance numbers taking with a single-threaded execution (and cleared cache) on boost:
boost 1thread before 130s, eb 22.7s, qlclass 8.7s

boost 1thread after 88s, eb 0s, qlclass 1.5s

Showing a considerable speedup in the two predicates that are modified in this PR.

@jbj jbj merged commit 3342fac into github:main Nov 20, 2020
@criemen criemen deleted the printast-performance branch November 20, 2020 10:48
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.

2 participants