Better performance in event parser property_iterator

[wmatw]

Hi, I am using ETW (and krabs) very intensively in my code, I am parsing different events at a very high rate (filesystem, registry...) To increase the performance of the parsing I am using the helpful propery_iterator instead of calls to find_property to eliminate the parser.propertyCache_ population. (to not use the find_property function, I am caching the result of size_provider::get_property_size too, then I am using pointer arithmetic to access the data in UserData) I changed the member name_ in the class property in property.hpp used by the class property_iterator to be a const wchar_t* instead of a std::wstring. That permits me to reduce drastically the allocation rate of my code. I think this low-level infrastructure should postpone such conversion and give the choice to the consumer to transform it (in my case with c++17 string_view give me a way to modernly work without reallocation by example) What do you think? Mathieu

[zacbrown]

Hmmm good observation. Could you make a PR that includes the usage of string_view for us to discuss?

Thanks so much for your insight and help!

[swannman]

Would the fluent predicates be faster if we updated them to work with string_view as well?

[wmatw]

For the explicit specialization for string arrays of the property_is predicate, string_view removes the need of temporary string allocation in the operator ().

[swannman]

Cool. That could be a nice performance boost for high-event-rate producers.

[swannman]

@wmatw are you able to produce a PR or point us to an example demonstrating your proposed change?

[wmatw]

I think I will be able to work on it soon, c++ 17 will break support of older compilers, what will the approach? Macro or direct implementation?

[swannman]

Looking forward to it!

I’m okay with introducing C++ 17 language features into the codebase as long as we are still able to build using the latest version of Visual Studio 2017.

[zacbrown]

@wmatw - regarding implementation approach, unless absolutely necessary, I think avoiding macros would be ideal. Perhaps something modeled after the view_of method on the parser type?

https://github.com/Microsoft/krabsetw/blob/0b42d12c337d2ea954ebc89c1547be48a552cf13/krabs/krabs/parser.hpp#L357

[zacbrown]

@swannman - I don't see any examples of its use publicly, but I seem to recall @kallanreed added view_of for use internally at MSFT. If possible, is there anything useful to share there on its actual usage? Mainly, an implementation of that Adapter type would be helpful. Something we can build an example out of will do.

[wmatw]

/std:c++17 option is not enabled by default on vs2017, adding include to will make it mandatory for the consumers of krabs. we can using #if __has_include() and make alternatives but it will cause code bloating, it will hurt readiness and complicate unit tests.

[kylereedmsft]

I don't love string_view and requiring c++17 mode for basically a pair of pointers seems like overkill.

You could use a template alias for collection_view<T> which is essentially a "span" type. https://github.com/Microsoft/krabsetw/blob/16580008afeeed887b4f2e61b523fb089f76a9df/krabs/krabs/collection_view.hpp

you could using ansi_string = collection_view<const char*>; then use the view overload that takes a pointer and length.

Or, I just noticed there's generic_string which is basically a string_view. What you'd need to watch out for is that you don't break the type-dispatch in the parser (it relies on the specialization of the overloads).

[swannman]

Revisiting this since it has been open for a year -- @kylereedmsft @wmatw any new thoughts on how best to implement this? Is collection_view<T> still the best approach, or is it an appropriate time for us to take a C++17 dependency?