-
Notifications
You must be signed in to change notification settings - Fork 487
Add compiler optimization to remove px.contains calls with empty strings
#1679
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
Merged
JamesMBartlett
merged 13 commits into
pixie-io:main
from
ddelnano:ddelnano/add-contains-compiler-optimization
Aug 21, 2023
Merged
Add compiler optimization to remove px.contains calls with empty strings
#1679
JamesMBartlett
merged 13 commits into
pixie-io:main
from
ddelnano:ddelnano/add-contains-compiler-optimization
Aug 21, 2023
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…return false Test Plan: - New unit tests pass - [ ] Verify in a staging environment that a query explain shows the nodes removed Signed-off-by: Dom Del Nano <ddelnano@newrelic.com> Differential Revision: https://phab.corp.pixielabs.ai/D12911 (cherry picked from commit 555043957b94f16e2163d1429b6f979d5fe77cec)
Signed-off-by: Dom Del Nano <ddelnano@pixielabs.ai>
Signed-off-by: Dom Del Nano <ddelnano@pixielabs.ai>
Signed-off-by: Dom Del Nano <ddelnano@pixielabs.ai>
Signed-off-by: Dom Del Nano <ddelnano@pixielabs.ai>
…ains are pruned Signed-off-by: Dom Del Nano <ddelnano@pixielabs.ai>
Signed-off-by: Dom Del Nano <ddelnano@pixielabs.ai>
… correct behavior Signed-off-by: Dom Del Nano <ddelnano@pixielabs.ai>
Signed-off-by: Dom Del Nano <ddelnano@pixielabs.ai>
Signed-off-by: Dom Del Nano <ddelnano@pixielabs.ai>
…hange Signed-off-by: Dom Del Nano <ddelnano@pixielabs.ai>
Member
Author
|
Moved this to a draft since I didn't expect to run into the sanitization issues and bugs with the GCC build. @pixie-io/maintainers would appreciate a review to see if you have any comments in addition to the fixes I need to get a passing build. |
Signed-off-by: Dom Del Nano <ddelnano@pixielabs.ai>
b4bfd92 to
73c4fd7
Compare
Signed-off-by: Dom Del Nano <ddelnano@pixielabs.ai>
73c4fd7 to
3274a56
Compare
JamesMBartlett
approved these changes
Aug 21, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary: Add compiler optimization to remove
px.containscalls with empty stringsThe majority of our pxl scripts have parameters that default to empty strings. This results in
px.containsfunction calls that will always return the original data set (an empty string""will always be found in any input string). An example of this can be seen in the following screenshot (the filter arguments highlighted below):Relevant Issues: Fixes #618
Type of change: /kind feature
Test Plan: Used analyze and explain query options to test the following conditions:
http_datapxl script looks like when there are no arguments supplied without any optimizations -- graphviz_with_empty_containshttp_datapxl script looks like if thepx.containscalls here are removed from the script. The optimization should produce the same graph as this case. -- graphviz_without_empty_contains