-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Data flow: Fix bad join-order when getAReadContent has large fan-in #10577
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Data flow: Fix bad join-order when getAReadContent has large fan-in #10577
Conversation
Before (terminated before completion) ``` Evaluated relational algebra for predicate DataFlowImplForHttpClientLibraries#c536b619::store#5#fffff@e5ef07bh with tuple counts: 151500 ~0% {4} r1 = SCAN DataFlowImplCommon#4f8df883::Cached::store#4#ffff OUTPUT In.1, In.0, In.2, In.3 150500 ~0% {5} r2 = JOIN r1 WITH DataFlowImplCommon#4f8df883::Cached::MkTypedContent#fff_20#join_rhs ON FIRST 1 OUTPUT Lhs.1, Lhs.0, Lhs.2, Lhs.3, Rhs.1 149500 ~0% {5} r3 = JOIN r2 WITH num#DataFlowImplForHttpClientLibraries#c536b619::TNodeNormal#ff ON FIRST 1 OUTPUT Lhs.2, Lhs.1, Lhs.3, Lhs.4, Rhs.1 148500 ~0% {5} r4 = JOIN r3 WITH num#DataFlowImplForHttpClientLibraries#c536b619::TNodeNormal#ff ON FIRST 1 OUTPUT Lhs.3, Lhs.1, Lhs.2, Lhs.4, Rhs.1 2003849000 ~0% {5} r5 = JOIN r4 WITH DataFlowPublic#e1781e31::ContentSet::getAReadContent#0#dispred#ff_10#join_rhs ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.2, Lhs.3, Lhs.4 105066500 ~9036% {5} r6 = JOIN r5 WITH project#DataFlowImplForHttpClientLibraries#c536b619::readSet#4#ffff ON FIRST 1 OUTPUT Lhs.3, Lhs.1, Lhs.4, Lhs.2, Rhs.1 return r6 ``` After ``` Evaluated relational algebra for predicate DataFlowImplForHttpClientLibraries#c536b619::readProj#2#ff@302620cn with tuple counts: 1461867 ~0% {2} r1 = SCAN DataFlowPrivate#462ff392::Cached::TContent#f OUTPUT In.0, In.0 3549054 ~1% {2} r2 = JOIN r1 WITH DataFlowPublic#e1781e31::ContentSet::getAReadContent#0#dispred#ff_10#join_rhs ON FIRST 1 OUTPUT Rhs.1, Lhs.1 5772824 ~5% {2} r3 = JOIN r2 WITH project#DataFlowImplForHttpClientLibraries#c536b619::readSet#4#ffff ON FIRST 1 OUTPUT Lhs.1, Rhs.1 return r3 Evaluated relational algebra for predicate DataFlowImplForHttpClientLibraries#c536b619::store#5#fffff@016cd9o1 with tuple counts: 267905 ~0% {4} r1 = SCAN DataFlowImplCommon#4f8df883::Cached::store#4#ffff OUTPUT In.1, In.0, In.2, In.3 267905 ~0% {5} r2 = JOIN r1 WITH DataFlowImplCommon#4f8df883::Cached::MkTypedContent#fff_20#join_rhs ON FIRST 1 OUTPUT Lhs.1, Lhs.0, Lhs.2, Lhs.3, Rhs.1 267905 ~0% {5} r3 = JOIN r2 WITH num#DataFlowImplForHttpClientLibraries#c536b619::TNodeNormal#ff ON FIRST 1 OUTPUT Lhs.2, Lhs.1, Lhs.3, Lhs.4, Rhs.1 267905 ~0% {5} r4 = JOIN r3 WITH num#DataFlowImplForHttpClientLibraries#c536b619::TNodeNormal#ff ON FIRST 1 OUTPUT Lhs.3, Lhs.1, Lhs.2, Lhs.4, Rhs.1 2109240 ~0% {5} r5 = JOIN r4 WITH DataFlowImplForHttpClientLibraries#c536b619::readProj#2#ff ON FIRST 1 OUTPUT Lhs.3, Lhs.1, Lhs.4, Lhs.2, Rhs.1 return r5 ```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C# LGTM 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Java looks plausible to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C++: Change seems reasonable. The DCA run looks OK (analysis performance most likely unchanged), though there don't seem to be the full set of nightly projects in the run.
cpp/ql/lib/experimental/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll
Outdated
Show resolved
Hide resolved
Java dca has apparently failed, but that's ok as Java currently don't use read-sets, and the change looks very safe. |
335e1a8
Merge commit: df2b586
Merge commit: df2b586
Merge commit: df2b586
Merge commit: df2b586
Before (terminated before completion)
After
Discovered on #10574, where a new
ContentSet
kind is introduced, which has a large fan-in throughgetAReadContent
(the specialTUnknownElementContent()
can be mapped toTKnownOrUnknownElementContent(c)
viagetAReadContent
, for allKnownElementContent c
).