Add back option to select preferred regex engine

I've seen some crashes in hyperscan internals. Added an option in settings to use only Qt engine as it is more stable.
variar · May 4, 2021 · a22c7f9 · a22c7f9
1 parent 9826b62
commit a22c7f9
Show file tree

Hide file tree

Showing 6 changed files with 114 additions and 29 deletions.
diff --git a/src/logdata/src/logfiltereddataworker.cpp b/src/logdata/src/logfiltereddataworker.cpp
@@ -99,7 +99,7 @@ PartialSearchResults filterLines( const MatcherVariant& matcher, const std::vect
         const auto& line = lines[ offset.get() ];
 
         const auto hasMatch
-            = absl::visit( [ &line ]( const auto& m ) { return m.hasMatch( line ); }, matcher );
+            = absl::visit( [&line]( const auto& m ) { return m.hasMatch( line ); }, matcher );
 
         if ( hasMatch ) {
             results.maxLength = qMax( results.maxLength, getUntabifiedLength( line ) );
@@ -146,7 +146,7 @@ void SearchData::addAll( LineLength length, const SearchResultArray& matches, Li
     if ( !matches.empty() ) {
         lastMatchedLineNumber_ = std::max( lastMatchedLineNumber_, matches.back().lineNumber() );
 
-        const auto insertNewMatches = [ &matches ]( SearchResultArray& oldMatches ) {
+        const auto insertNewMatches = [&matches]( SearchResultArray& oldMatches ) {
             const auto insertIt
                 = std::lower_bound( begin( oldMatches ), end( oldMatches ), matches.front() );
 
@@ -258,7 +258,7 @@ void LogFilteredDataWorker::search( const RegularExpressionPattern& regExp, Line
     operationFuture_.waitForFinished();
     interruptRequested_.clear();
 
-    operationFuture_ = QtConcurrent::run( [ this, regExp, startLine, endLine ] {
+    operationFuture_ = QtConcurrent::run( [this, regExp, startLine, endLine] {
         auto operationRequested = std::make_unique<FullSearchOperation>(
             sourceLogData_, interruptRequested_, regExp, startLine, endLine );
         connectSignalsAndRun( operationRequested.get() );
@@ -276,7 +276,7 @@ void LogFilteredDataWorker::updateSearch( const RegularExpressionPattern& regExp
     operationFuture_.waitForFinished();
     interruptRequested_.clear();
 
-    operationFuture_ = QtConcurrent::run( [ this, regExp, startLine, endLine, position ] {
+    operationFuture_ = QtConcurrent::run( [this, regExp, startLine, endLine, position] {
         auto operationRequested = std::make_unique<UpdateSearchOperation>(
             sourceLogData_, interruptRequested_, regExp, startLine, endLine, position );
         connectSignalsAndRun( operationRequested.get() );
@@ -322,7 +322,7 @@ void SearchOperation::doSearch( SearchData& searchData, LineNumber initialLine )
     high_resolution_clock::time_point t1 = high_resolution_clock::now();
 
     const auto& config = Configuration::get();
-    const auto matchingThreadsCount = static_cast<uint32_t>( [ &config ]() {
+    const auto matchingThreadsCount = static_cast<uint32_t>( [&config]() {
         if ( !config.useParallelSearch() ) {
             return 1;
         }
@@ -352,8 +352,8 @@ void SearchOperation::doSearch( SearchData& searchData, LineNumber initialLine )
     auto chunkStart = initialLine;
     auto linesSource = tbb::flow::input_node<BlockDataType>(
         searchGraph,
-        [ this, endLine, nbLinesInChunk, &chunkStart,
-          &fileReadingDuration ]( tbb::flow_control& fc ) -> BlockDataType {
+        [this, endLine, nbLinesInChunk, &chunkStart,
+         &fileReadingDuration]( tbb::flow_control& fc ) -> BlockDataType {
             if ( interruptRequested_ ) {
                 LOG_INFO << "Block reader interrupted";
                 fc.stop();
@@ -399,19 +399,20 @@ void SearchOperation::doSearch( SearchData& searchData, LineNumber initialLine )
 
     auto lineBlocksQueue = tbb::flow::buffer_node<BlockDataType>( searchGraph );
 
-    HsRegularExpression hsRegularExpression{ regexp_ };
-
     using RegexMatcherNode
         = tbb::flow::function_node<BlockDataType, PartialResultType, tbb::flow::rejecting>;
-
     using MatcherContext = std::tuple<MatcherVariant, microseconds, RegexMatcherNode>;
+
     std::vector<MatcherContext> regexMatchers;
+    HsRegularExpression hsRegularExpression{ regexp_ };
+    const auto useHyperscanEngine = config.regexpEngine() == RegexpEngine::Hyperscan;
     for ( auto index = 0u; index < matchingThreadsCount; ++index ) {
         regexMatchers.emplace_back(
-            hsRegularExpression.createMatcher(),
+            useHyperscanEngine ? hsRegularExpression.createMatcher()
+                               : MatcherVariant{ DefaultRegularExpressionMatcher( regexp_ ) },
             microseconds{ 0 },
             RegexMatcherNode(
-                searchGraph, 1, [ this, &regexMatchers, index ]( const BlockDataType& blockData ) {
+                searchGraph, 1, [this, &regexMatchers, index]( const BlockDataType& blockData ) {
                     const auto& matcher = std::get<0>( regexMatchers.at( index ) );
                     const auto matchStartTime = high_resolution_clock::now();
 
@@ -442,7 +443,7 @@ void SearchOperation::doSearch( SearchData& searchData, LineNumber initialLine )
 
     auto matchProcessor = tbb::flow::function_node<PartialResultType, tbb::flow::continue_msg,
                                                    tbb::flow::rejecting>(
-        searchGraph, 1, [ & ]( const PartialResultType& matchResults ) {
+        searchGraph, 1, [&]( const PartialResultType& matchResults ) {
             if ( interruptRequested_ ) {
                 LOG_INFO << "Match processor interrupted";
                 return tbb::flow::continue_msg{};

diff --git a/src/ui/include/optionsdialog.h b/src/ui/include/optionsdialog.h
@@ -80,6 +80,9 @@ class OptionsDialog : public QDialog, public Ui::OptionsDialog {
     int getRegexpTypeIndex( SearchRegexpType syntax ) const;
     SearchRegexpType getRegexpTypeFromIndex( int index ) const;
 
+    int getRegexpEngineIndex( RegexpEngine engine ) const;
+    RegexpEngine getRegexpEngineFromIndex( int index ) const;
+
     void updateDialogFromConfig();
 };
 

diff --git a/src/ui/include/optionsdialog.ui b/src/ui/include/optionsdialog.ui
@@ -7,7 +7,7 @@
     <x>0</x>
     <y>0</y>
     <width>617</width>
-    <height>711</height>
+    <height>619</height>
    </rect>
   </property>
   <property name="sizePolicy">
@@ -468,13 +468,30 @@
            <set>Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter</set>
           </property>
           <item row="0" column="0">
+           <widget class="QLabel" name="regexpEngineLabel">
+            <property name="text">
+             <string>Regular expressions engine:</string>
+            </property>
+           </widget>
+          </item>
+          <item row="3" column="0">
+           <widget class="QCheckBox" name="parallelSearchCheckBox">
+            <property name="text">
+             <string>Use parallel search</string>
+            </property>
+            <property name="checked">
+             <bool>true</bool>
+            </property>
+           </widget>
+          </item>
+          <item row="1" column="0">
            <widget class="QLabel" name="label_8">
             <property name="text">
              <string>Index file read buffer (Mib):</string>
             </property>
            </widget>
           </item>
-          <item row="0" column="1">
+          <item row="1" column="1">
            <widget class="QSpinBox" name="indexReadBufferSpinBox">
             <property name="sizePolicy">
              <sizepolicy hsizetype="MinimumExpanding" vsizetype="Fixed">
@@ -484,14 +501,14 @@
             </property>
            </widget>
           </item>
-          <item row="1" column="0">
+          <item row="2" column="0">
            <widget class="QLabel" name="label_9">
             <property name="text">
              <string>Search read buffer (lines):</string>
             </property>
            </widget>
           </item>
-          <item row="1" column="1">
+          <item row="2" column="1">
            <widget class="QSpinBox" name="searchReadBufferSpinBox">
             <property name="sizePolicy">
              <sizepolicy hsizetype="MinimumExpanding" vsizetype="Fixed">
@@ -507,17 +524,7 @@
             </property>
            </widget>
           </item>
-          <item row="2" column="0">
-           <widget class="QCheckBox" name="parallelSearchCheckBox">
-            <property name="text">
-             <string>Use parallel search</string>
-            </property>
-            <property name="checked">
-             <bool>true</bool>
-            </property>
-           </widget>
-          </item>
-          <item row="3" column="0">
+          <item row="4" column="0">
            <widget class="QCheckBox" name="keepFileClosedCheckBox">
             <property name="toolTip">
              <string>File will be kept closed as much as possible. Affects only files opened after check state changed</string>
@@ -527,6 +534,16 @@
             </property>
            </widget>
           </item>
+          <item row="0" column="1">
+           <widget class="QComboBox" name="regexpEngineComboBox">
+            <property name="sizePolicy">
+             <sizepolicy hsizetype="MinimumExpanding" vsizetype="Fixed">
+              <horstretch>0</horstretch>
+              <verstretch>0</verstretch>
+             </sizepolicy>
+            </property>
+           </widget>
+          </item>
          </layout>
         </widget>
        </item>

diff --git a/src/ui/src/optionsdialog.cpp b/src/ui/src/optionsdialog.cpp
@@ -97,6 +97,11 @@ void OptionsDialog::setupTabs()
 #ifdef Q_OS_MAC
     minimizeToTrayCheckBox->setVisible( false );
 #endif
+
+#ifndef KLOGG_HAS_HS
+    regexpEngineLabel->setVisible( false );
+    regexpEngineComboBox->setVisible( false );
+#endif
 }
 
 // Populates the 'family' ComboBox
@@ -120,6 +125,11 @@ void OptionsDialog::setupRegexp()
 
     mainSearchBox->addItems( regexpTypes );
     quickFindSearchBox->addItems( regexpTypes );
+
+    QStringList regexpEngines;
+    regexpEngines << tr( "Hyperscan" ) << tr( "Qt" );
+
+    regexpEngineComboBox->addItems( regexpEngines );
 }
 
 void OptionsDialog::setupStyles()
@@ -194,6 +204,38 @@ SearchRegexpType OptionsDialog::getRegexpTypeFromIndex( int index ) const
     return type;
 }
 
+int OptionsDialog::getRegexpEngineIndex( RegexpEngine engine ) const
+{
+    int index;
+
+    switch ( engine ) {
+    case RegexpEngine::QRegularExpression:
+        index = 1;
+        break;
+    default:
+        index = 0;
+        break;
+    }
+
+    return index;
+}
+
+RegexpEngine OptionsDialog::getRegexpEngineFromIndex( int index ) const
+{
+    RegexpEngine type;
+
+    switch ( index ) {
+    case 1:
+        type = RegexpEngine::QRegularExpression;
+        break;
+    default:
+        type = RegexpEngine::Hyperscan;
+        break;
+    }
+
+    return type;
+}
+
 // Updates the dialog box using values in global Config()
 void OptionsDialog::updateDialogFromConfig()
 {
@@ -227,6 +269,7 @@ void OptionsDialog::updateDialogFromConfig()
     // Regexp types
     mainSearchBox->setCurrentIndex( getRegexpTypeIndex( config.mainRegexpType() ) );
     quickFindSearchBox->setCurrentIndex( getRegexpTypeIndex( config.quickfindRegexpType() ) );
+    regexpEngineComboBox->setCurrentIndex( getRegexpEngineIndex( config.regexpEngine() ) );
 
     incrementalCheckBox->setChecked( config.isQuickfindIncremental() );
 
@@ -301,6 +344,7 @@ void OptionsDialog::updateConfigFromDialog()
     config.setMainRegexpType( getRegexpTypeFromIndex( mainSearchBox->currentIndex() ) );
     config.setQuickfindRegexpType( getRegexpTypeFromIndex( quickFindSearchBox->currentIndex() ) );
     config.setQuickfindIncremental( incrementalCheckBox->isChecked() );
+    config.setRegexpEnging( getRegexpEngineFromIndex( regexpEngineComboBox->currentIndex() ) );
 
     config.setNativeFileWatchEnabled( nativeFileWatchCheckBox->isChecked() );
     config.setPollingEnabled( pollingCheckBox->isChecked() );

diff --git a/src/utils/include/configuration.h b/src/utils/include/configuration.h
@@ -52,6 +52,11 @@ enum class SearchRegexpType {
     FixedString,
 };
 
+enum class RegexpEngine {
+    Hyperscan,
+    QRegularExpression
+};
+
 // Configuration class containing everything in the "Settings" dialog
 class Configuration final : public Persistable<Configuration> {
   public:
@@ -224,6 +229,16 @@ class Configuration final : public Persistable<Configuration> {
         useLineEndingCache_ = useLineEndingCache;
     }
 
+    RegexpEngine regexpEngine() const
+    {
+        return regexpEngine_;
+    }
+
+    void setRegexpEnging( RegexpEngine engine )
+    {
+        regexpEngine_ = engine;
+    }
+
     // Accessors
     bool versionCheckingEnabled() const
     {
@@ -437,6 +452,9 @@ class Configuration final : public Persistable<Configuration> {
     bool enableQtHighDpi_ = true;
 
     int scaleFactorRounding_ = 1;
+
+    RegexpEngine regexpEngine_ = RegexpEngine::Hyperscan;
+
 };
 
 #endif
diff --git a/src/utils/src/configuration.cpp b/src/utils/src/configuration.cpp
@@ -101,7 +101,8 @@ void Configuration::retrieveFromStorage( QSettings& settings )
     quickfindRegexpType_ = static_cast<SearchRegexpType>(
         settings.value( "regexpType.quickfind", static_cast<int>( Default.quickfindRegexpType_ ) )
             .toInt() );
-
+    regexpEngine_ = static_cast<RegexpEngine>(
+        settings.value( "regexpType.engine", static_cast<int>( Default.regexpEngine_ ) ).toInt() );
     quickfindIncremental_
         = settings.value( "quickfind.incremental", Default.quickfindIncremental_ ).toBool();
 
@@ -214,6 +215,7 @@ void Configuration::saveToStorage( QSettings& settings ) const
     settings.setValue( "mainFont.antialiasing", forceFontAntialiasing_ );
     settings.setValue( "regexpType.main", static_cast<int>( mainRegexpType_ ) );
     settings.setValue( "regexpType.quickfind", static_cast<int>( quickfindRegexpType_ ) );
+    settings.setValue( "regexpType.engine", static_cast<int>( regexpEngine_ ) );
     settings.setValue( "quickfind.incremental", quickfindIncremental_ );
     settings.setValue( "filewatch.useNative", nativeFileWatchEnabled_ );
     settings.setValue( "filewatch.usePolling", pollingEnabled_ );