[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #928: MINIFICPP-1394 - Speed up ConsumeWindowsEventLog processor

[GitBox]

adamdebreceni commented on a change in pull request #928:
URL: https://github.com/apache/nifi-minifi-cpp/pull/928#discussion_r509023703



##
File path: extensions/windows-event-log/wel/WindowsEventLog.cpp
##
@@ -43,10 +45,10 @@ void WindowsEventLogMetadataImpl::renderMetadata() {
   const auto contextGuard = gsl::finally([](){
 EvtClose(context);
   });
-  if (!EvtRender(context, event_ptr_, EvtRenderEventValues, dwBufferSize, 
nullptr, , )) {
+  if (!EvtRender(context, event_ptr_, EvtRenderEventValues, dwBufferSize, 
rendered_values.get(), , )) {
 if (ERROR_INSUFFICIENT_BUFFER == (status = GetLastError())) {
   dwBufferSize = dwBufferUsed;
-  rendered_values = std::unique_ptr((PEVT_VARIANT)(malloc(dwBufferSize)));
+  rendered_values.reset((PEVT_VARIANT)(malloc(dwBufferSize)));

Review comment:
   if you check all api calls, you can see that it first assigns the 
required buffer size (here `dwBufferUsed `) to the variable storing the actual 
buffer size (here `dwBufferSize `) and then calls `malloc` with the actual 
buffer size (modulo if it is in bytes or chars)
   
   do you think it would be an improvement in readability if we allocate based 
on the required buffer size and then assign it to the actual buffer size?





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #928: MINIFICPP-1394 - Speed up ConsumeWindowsEventLog processor

[GitBox]

adamdebreceni commented on a change in pull request #928:
URL: https://github.com/apache/nifi-minifi-cpp/pull/928#discussion_r509021609



##
File path: extensions/windows-event-log/wel/WindowsEventLog.cpp
##
@@ -129,34 +133,30 @@ std::string 
WindowsEventLogMetadataImpl::getEventData(EVT_FORMAT_MESSAGE_FLAGS f
 return event_data;
   }
 
-  if (!EvtFormatMessage(metadata_ptr_, event_ptr_, 0, 0, NULL, flags, 
string_buffer_size, string_buffer, _buffer_used)) {
+
+  if (!EvtFormatMessage(metadata_ptr_, event_ptr_, 0, 0, NULL, flags, 
string_buffer_size, string_buffer.get(), _buffer_used)) {
 result = GetLastError();
 if (ERROR_INSUFFICIENT_BUFFER == result) {
   string_buffer_size = string_buffer_used;
 
-  string_buffer = (LPWSTR) malloc(string_buffer_size * sizeof(WCHAR));
-
-  if (string_buffer) {
+  string_buffer.reset((LPWSTR) malloc(string_buffer_size * sizeof(WCHAR)));
 
-if ((EvtFormatMessageKeyword == flags))
-  string_buffer[string_buffer_size - 1] = L'\0';
-
-EvtFormatMessage(metadata_ptr_, event_ptr_, 0, 0, NULL, flags, 
string_buffer_size, string_buffer, _buffer_used);
-if ((EvtFormatMessageKeyword == flags))
-  string_buffer[string_buffer_used - 1] = L'\0';
-std::wstring str(string_buffer);
-event_data = std::string(str.begin(), str.end());
-free(string_buffer);
-  }
+  EvtFormatMessage(metadata_ptr_, event_ptr_, 0, 0, NULL, flags, 
string_buffer_size, string_buffer.get(), _buffer_used);
 }
   }
+  if ((EvtFormatMessageKeyword == flags))

Review comment:
   done

##
File path: extensions/windows-event-log/ConsumeWindowsEventLog.cpp
##
@@ -514,30 +515,33 @@ void 
ConsumeWindowsEventLog::substituteXMLPercentageItems(pugi::xml_document& do
 
 bool ConsumeWindowsEventLog::createEventRender(EVT_HANDLE hEvent, EventRender& 
eventRender) {
   logger_->log_trace("Rendering an event");
-  DWORD size = 0;
+  DWORD size = sizeof(WCHAR) * 4096;
+  WCHAR stackBuffer[4096];
+  using Deleter = utils::StackAwareDeleter;
+  std::unique_ptr buf{stackBuffer, Deleter{ stackBuffer }};
+
   DWORD used = 0;
   DWORD propertyCount = 0;
-  EvtRender(NULL, hEvent, EvtRenderEventXml, size, 0, , );
-  if (ERROR_INSUFFICIENT_BUFFER != GetLastError()) {
-LOG_LAST_ERROR(EvtRender);
-return false;
-  }
-
-  if (used > maxBufferSize_) {
-logger_->log_error("Dropping event because it couldn't be rendered within 
%" PRIu64 " bytes.", maxBufferSize_);
-return false;
-  }
-
-  size = used;
-  std::vector buf(size / 2 + 1);
-  if (!EvtRender(NULL, hEvent, EvtRenderEventXml, size, [0], , 
)) {
-LOG_LAST_ERROR(EvtRender);
-return false;
+  if (!EvtRender(NULL, hEvent, EvtRenderEventXml, size, buf.get(), , 
)) {
+if (ERROR_INSUFFICIENT_BUFFER != GetLastError()) {
+  LOG_LAST_ERROR(EvtRender);
+  return false;
+}
+if (used > maxBufferSize_) {
+  logger_->log_error("Dropping event because it couldn't be rendered 
within %" PRIu64 " bytes.", maxBufferSize_);
+  return false;
+}
+size = used;
+buf.reset((LPWSTR)malloc(size));

Review comment:
   done

##
File path: extensions/windows-event-log/wel/WindowsEventLog.cpp
##
@@ -129,34 +133,30 @@ std::string 
WindowsEventLogMetadataImpl::getEventData(EVT_FORMAT_MESSAGE_FLAGS f
 return event_data;
   }
 
-  if (!EvtFormatMessage(metadata_ptr_, event_ptr_, 0, 0, NULL, flags, 
string_buffer_size, string_buffer, _buffer_used)) {
+
+  if (!EvtFormatMessage(metadata_ptr_, event_ptr_, 0, 0, NULL, flags, 
string_buffer_size, string_buffer.get(), _buffer_used)) {
 result = GetLastError();
 if (ERROR_INSUFFICIENT_BUFFER == result) {
   string_buffer_size = string_buffer_used;
 
-  string_buffer = (LPWSTR) malloc(string_buffer_size * sizeof(WCHAR));
-
-  if (string_buffer) {
+  string_buffer.reset((LPWSTR) malloc(string_buffer_size * sizeof(WCHAR)));
 
-if ((EvtFormatMessageKeyword == flags))
-  string_buffer[string_buffer_size - 1] = L'\0';
-
-EvtFormatMessage(metadata_ptr_, event_ptr_, 0, 0, NULL, flags, 
string_buffer_size, string_buffer, _buffer_used);
-if ((EvtFormatMessageKeyword == flags))
-  string_buffer[string_buffer_used - 1] = L'\0';
-std::wstring str(string_buffer);
-event_data = std::string(str.begin(), str.end());
-free(string_buffer);
-  }
+  EvtFormatMessage(metadata_ptr_, event_ptr_, 0, 0, NULL, flags, 
string_buffer_size, string_buffer.get(), _buffer_used);
 }
   }
+  if ((EvtFormatMessageKeyword == flags))
+string_buffer.get()[string_buffer_used - 1] = L'\0';
+  std::wstring str(string_buffer.get());
+  event_data = std::string(str.begin(), str.end());
   return event_data;
 }
 
 std::string WindowsEventLogHandler::getEventMessage(EVT_HANDLE eventHandle) 
const {
   std::string returnValue;
-  std::unique_ptr pBuffer;
-  DWORD dwBufferSize = 0;
+  DWORD 

[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #928: MINIFICPP-1394 - Speed up ConsumeWindowsEventLog processor

[GitBox]

adamdebreceni commented on a change in pull request #928:
URL: https://github.com/apache/nifi-minifi-cpp/pull/928#discussion_r509021203



##
File path: extensions/windows-event-log/ConsumeWindowsEventLog.cpp
##
@@ -514,30 +515,33 @@ void 
ConsumeWindowsEventLog::substituteXMLPercentageItems(pugi::xml_document& do
 
 bool ConsumeWindowsEventLog::createEventRender(EVT_HANDLE hEvent, EventRender& 
eventRender) {
   logger_->log_trace("Rendering an event");
-  DWORD size = 0;
+  DWORD size = sizeof(WCHAR) * 4096;
+  WCHAR stackBuffer[4096];
+  using Deleter = utils::StackAwareDeleter;
+  std::unique_ptr buf{stackBuffer, Deleter{ stackBuffer }};
+
   DWORD used = 0;
   DWORD propertyCount = 0;
-  EvtRender(NULL, hEvent, EvtRenderEventXml, size, 0, , );
-  if (ERROR_INSUFFICIENT_BUFFER != GetLastError()) {
-LOG_LAST_ERROR(EvtRender);
-return false;
-  }
-
-  if (used > maxBufferSize_) {
-logger_->log_error("Dropping event because it couldn't be rendered within 
%" PRIu64 " bytes.", maxBufferSize_);
-return false;
-  }
-
-  size = used;
-  std::vector buf(size / 2 + 1);
-  if (!EvtRender(NULL, hEvent, EvtRenderEventXml, size, [0], , 
)) {
-LOG_LAST_ERROR(EvtRender);
-return false;
+  if (!EvtRender(NULL, hEvent, EvtRenderEventXml, size, buf.get(), , 
)) {
+if (ERROR_INSUFFICIENT_BUFFER != GetLastError()) {
+  LOG_LAST_ERROR(EvtRender);
+  return false;
+}
+if (used > maxBufferSize_) {
+  logger_->log_error("Dropping event because it couldn't be rendered 
within %" PRIu64 " bytes.", maxBufferSize_);
+  return false;
+}
+size = used;
+buf.reset((LPWSTR)malloc(size));
+if (!EvtRender(NULL, hEvent, EvtRenderEventXml, size, buf.get(), , 
)) {
+  LOG_LAST_ERROR(EvtRender);
+  return false;
+}
   }
 
   logger_->log_debug("Event rendered with size %" PRIu32 ". Performing doc 
traversing...", size);

Review comment:
   done

##
File path: extensions/windows-event-log/wel/WindowsEventLog.cpp
##
@@ -118,8 +120,10 @@ void WindowsEventLogMetadataImpl::renderMetadata() {
 }
 
 std::string WindowsEventLogMetadataImpl::getEventData(EVT_FORMAT_MESSAGE_FLAGS 
flags) const {
-  LPWSTR string_buffer = NULL;
-  DWORD string_buffer_size = 0;
+  DWORD string_buffer_size = 4096;

Review comment:
   done





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org