Skip to content
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

Add a trace interface to enable ignoring MOF events #210

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

acyr
Copy link
Contributor

@acyr acyr commented May 2, 2023

MOF events all require a (slow) TDH lookup which can fail if schema is not available. Since these are generally legacy events, many applications may not require them and ignoring them can provide a significant performance increase, particularly for large trace files.

MOF events all require a (slow) TDH lookup which can fail if schema is
not available.  Since these are generally legacy events, many
applications may not require them and ignoring them can provide a
significant performance increase, particularly for large trace files.
@acyr
Copy link
Contributor Author

acyr commented May 2, 2023

Ignoring MOF events for some ETL traces provide orders of magnitudes speed improvements.

Real Example: ~500MB ETL trace is parsed in 146s with MOF events -> 6s without MOF events.

Problem seems rooted in that reading from an ETL file does not seem to obey event filters, making it possible for undesired events to be read in regardless of provider filter settings.

Copy link
Member

@swannman swannman left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this! I had a bit of feedback on naming but otherwise this looks good.

* trace.set_ignore_mof_events(true);
* </example>
*/
void set_ignore_mof_events(bool ignore_mof_events);
Copy link
Member

Choose a reason for hiding this comment

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

Let's call this set_mof_event_processing_enabled(bool mof_events_enabled) with a default of true to avoid a double-negative.

@@ -308,6 +322,8 @@ namespace krabs {

provider_callback default_callback_ = nullptr;

bool ignore_mof_events_ = false;
Copy link
Member

Choose a reason for hiding this comment

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

mof_events_enabled_ = true;

@swannman
Copy link
Member

@acyr We're happy to merge this pending the feedback above. Are you still working on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants