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

Better multithreading code #1394

Merged
merged 4 commits into from Jun 8, 2018

Conversation

Projects
None yet
4 participants
@MKleusberg
Copy link
Member

MKleusberg commented May 21, 2018

This is the patch which Michael Krause posted here. I have changed his code to apply cleanly on the current master branch, fixed some compiler warnings and some whitespace errors. No major changes there - that's the first commit. Then I did some extra changes which I thought made sense and added them as extra commits.

It should compile and it should run, so please give it a test if you can. There are still some "TODO" comments in the code and they definitely need to be addressed but the new code is a huge improvement in my opinion. But I haven't yet tested this thoroughly.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented May 21, 2018

Looks like Travis infrastructure is having issues again: 😦

E: Failed to fetch http://mirror.jmu.edu/pub/ubuntu/dists/trusty-updates/universe/i18n/Translation-en
   Writing more data than expected (243993 > 243863)
@mgrojo

This comment has been minimized.

Copy link
Contributor

mgrojo commented May 21, 2018

I had to include <memory> header in sqlitetablemodel.h in order to compile it.

@MKleusberg

This comment has been minimized.

Copy link
Member Author

MKleusberg commented May 21, 2018

Thanks! For some reason I removed that include. It's fixed now. Travis decided to re-run the build, too, and complained about the same missing include.

@MKleusberg MKleusberg force-pushed the multithreading branch 2 times, most recently from b0066eb to ff6545f May 22, 2018

@MKleusberg

This comment has been minimized.

Copy link
Member Author

MKleusberg commented May 22, 2018

Here's a list of the TODOs mentioned in the patch by Michael Krause:

  • In MainWindow::populateTable(): actual call this once to disable Delete button
  • In PlotDock::fetchAllData(): make this cancellable & show progress
  • In RowCache.h: introduce maximum segment size?
  • In SqliteTableModel::handleFinishedFetch(): optimize

@MKleusberg MKleusberg force-pushed the multithreading branch from ff6545f to 35bcad0 May 22, 2018

@MKleusberg

This comment has been minimized.

Copy link
Member Author

MKleusberg commented May 22, 2018

Yay, Travis is finally reporting a success 🎆 So that's the first major step done towards merging this 😄

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented May 24, 2018

Not sure it's relevant, but these warnings show up when compiling this branch on OSX:

In file included from sqlitedb.cpp:3:
./sqlitetablemodel.h:88:10: warning: 'sort' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
    void sort(int column, Qt::SortOrder order = Qt::AscendingOrder);
         ^
../../../Qt/5.9.2/clang_64/lib/QtCore.framework/Headers/qabstractitemmodel.h:233:18: note: overridden virtual function is here
    virtual void sort(int column, Qt::SortOrder order = Qt::AscendingOrder);
                 ^
In file included from sqlitedb.cpp:3:
./sqlitetablemodel.h:91:19: warning: 'flags' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
    Qt::ItemFlags flags(const QModelIndex& index) const;
                  ^
../../../Qt/5.9.2/clang_64/lib/QtCore.framework/Headers/qabstractitemmodel.h:378:19: note: overridden virtual function is here
    Qt::ItemFlags flags(const QModelIndex &index) const Q_DECL_OVERRIDE;
                  ^
In file included from sqlitedb.cpp:3:
./sqlitetablemodel.h:119:29: warning: 'supportedDropActions' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
    virtual Qt::DropActions supportedDropActions() const;
                            ^
../../../Qt/5.9.2/clang_64/lib/QtCore.framework/Headers/qabstractitemmodel.h:204:29: note: overridden virtual function is here
    virtual Qt::DropActions supportedDropActions() const;
                            ^
In file included from sqlitedb.cpp:3:
./sqlitetablemodel.h:120:18: warning: 'dropMimeData' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
    virtual bool dropMimeData(const QMimeData* data, Qt::DropAction action, int row, int column, const QModelIndex& parent);
                 ^
../../../Qt/5.9.2/clang_64/lib/QtCore.framework/Headers/qabstractitemmodel.h:375:10: note: overridden virtual function is here
    bool dropMimeData(const QMimeData *data, Qt::DropAction action,
         ^
sqlitedb.cpp:542:12: warning: moving a local object in a return statement prevents copy elision [-Wpessimizing-move]
    return std::move(lk);
           ^
sqlitedb.cpp:542:12: note: remove std::move call here
    return std::move(lk);
           ^~~~~~~~~~  ~
@mkweskin

This comment has been minimized.

Copy link

mkweskin commented May 26, 2018

I've tried out the one-off OSX build. I loaded a large data set (~500K lines). I saw the significant improvements in responsiveness in switching to the "Browse Data" tab and improvements when I scrolled and jumped to an arbitrary place in the listings. I didn't see any weirdness.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented May 26, 2018

That bodes well. Thanks for helping @mkweskin. 😄

@mgrojo

This comment has been minimized.

Copy link
Contributor

mgrojo commented May 26, 2018

There is a problem in this branch with the filters. Every keystroke in the filter box makes the box loose the focus, which is annoying for the user. I've looked for something related to focus switch in the changes without success. I suppose something is triggered that ends in this focus loss.

@MKleusberg

This comment has been minimized.

Copy link
Member Author

MKleusberg commented May 26, 2018

@justinclift Those warning should be fixed with yesterday's commit. Nothing serious there but good to fix them anyway.

@mgrojo Good point. Turns out I only ever tested the filters in this branch with one-character search strings 😉 I believe the commit I have just pushed is the proper way to fix this and it definitely has less side effects than the other ideas I had for this. Still worth testing it though.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented May 26, 2018

@MKleusberg Thanks. I'll check in a bit and see if there were any others too. 😄

@mgrojo

This comment has been minimized.

Copy link
Contributor

mgrojo commented May 26, 2018

@MKleusberg The problem with the filters is solved now 👍

I've been testing it with a big database (an import of National_Statistics_Postcode_Lookup_UK.csv). It works really good. Sometimes I miss a bit of feedback about being working on it for the operations that take a bit of time, like the filters (only that "determining row count...", which can isn't very noticeable) or the plot.

@mgrojo

This comment has been minimized.

Copy link
Contributor

mgrojo commented May 26, 2018

I've remembered #1373 and, willing see how it behaves in this branch and fearing that it takes all my RAM, I've selected the entire table of the big postcode DB using the upper left corner. It indeed seems to work as expected because it was blocked for a long time. I left it alone and when I came back it had finished without message in standard output nor /var/log files. In the master branch it can select the visible rows, but it cannot copy and paste them to another application (it also gets blocked). I guess this branch solves the partial selection problem described in #1373, but it leads to a new question. Should we let the user select an entire big table without any warning?

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented May 26, 2018

Just tried out this new branch with the UK postcode database too.

Wow. This is a lot faster and more usable. Well done guys. 😄

Should we let the user select an entire big table without any warning?

I guess this is one of those things where we should do some measurements on the PC we have, to work out what size of data selection makes things slow.

Maybe we should add some debug logging (in a branch), to output stats to (say) a dialog or log file, and we can collect the stats info in an issue. Once we have some data to work with, we could figure out where the warning (or whatever) thresholds are.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented May 26, 2018

Hmmm, the improvement when working with larger data sets is pretty huge. I'm making new once-off builds for Win and OSX now, and we can point people at them via Twitter and some of the existing issues here. That should get people testing things. 😄

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented May 26, 2018

@MKleusberg Found something interesting when compiling this branch on Windows just now. It was erroring out with something along the lines of not being able to find pthread.lib.

Did a quick bit of poking around, and it looks like the new pthread line in CMakeLists.txt is causing the problem. With that removed (eg 1fceeca), then Win32 compiles ok. The Win64 build is still going atm, but I'm expecting that'll compile ok to.

Testing on CentOS 7 x64 just now... that compiles fine either way. Doesn't seem sensitive to pthread being included or not.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented May 26, 2018

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented May 26, 2018

Just had another thought for the "should we let users select a big table without warning?" thing.

Are we able to detect the start of the selection event, and if it runs for longer than (say) 3 seconds we pop up a warning dialog of some sort?

@justinclift justinclift referenced this pull request May 26, 2018

Closed

How to increase performance? #1364

3 of 10 tasks complete
@mgrojo

This comment has been minimized.

Copy link
Contributor

mgrojo commented May 26, 2018

@justinclift I was taking a look at old issues. Should we also notify the reporters of #761 and #503?

Are we able to detect the start of the selection event, and if it runs for longer than (say) 3 seconds we pop up a warning dialog of some sort?

I don't know if it's feasible, but it would be a solution.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented May 26, 2018

Thanks @mgrojo. #503 looks like a good candidate. I'll ping the people in that now.

#761 is performance related, but in a different area to this so we can skip contacting the people in that for this. We do need to verify if the fix already done (and merged) for #761 has completely solved that issue. Personally, I can't be bothered testing that one just now. 😉

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jun 7, 2018

@MKleusberg Just a minor reminder, we'll still need to do something about that pthread inclusion. At least on Windows anyway. 😄

@MKleusberg

This comment has been minimized.

Copy link
Member Author

MKleusberg commented Jun 7, 2018

@justinclift Oh yeah, completely forgot about that. I think we can just get rid of it entirely. But let's see what Travis says about that 😄

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jun 7, 2018

Damn. Looks like we'll be needing some #ifdef work:

[ 70%] Linking CXX executable sqlitebrowser
/usr/bin/ld: CMakeFiles/sqlitebrowser.dir/src/RowLoader.cpp.o: undefined reference to symbol 'pthread_create@@GLIBC_2.2.5'
//lib/x86_64-linux-gnu/libpthread.so.0: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
make[2]: *** [sqlitebrowser] Error 1
make[1]: *** [CMakeFiles/sqlitebrowser.dir/all] Error 2
make: *** [all] Error 2

@MKleusberg MKleusberg force-pushed the multithreading branch from 984f0a0 to d585024 Jun 7, 2018

@MKleusberg

This comment has been minimized.

Copy link
Member Author

MKleusberg commented Jun 7, 2018

Ok, now it builds 😄 @justinclift Can you give it a try on Windows to check if it works there too?

@MKleusberg MKleusberg force-pushed the multithreading branch from d585024 to 2e25b3f Jun 7, 2018

MKleusberg added some commits May 21, 2018

Multi-threading patch
This was done by Michael Krause.
https://lists.sqlitebrowser.org/pipermail/db4s-dev/2018-February/000305.html

In this commit I only fixed two compiler warnings, some whitespace
issues and removed some debug messages.
Clean up multi threading patch, fix build and some bugs
Make strings translatable, remove some more debug code, fix tests,
reduce size of patch slightly, remove weird tooltip, don't crash when
closing database, simplify code, fix filters, don't link agains pthread
on Windows.

@MKleusberg MKleusberg force-pushed the multithreading branch from 56f92f9 to b2b2fbd Jun 7, 2018

Tweak enabling/disabling of insert and delete record buttons
Improve the enabling and disabling code for the insert and delete record
buttons in the Browse Data tab. With this the buttons should be enabled
if and only if they can actually be used. The commit also makes the code
easier to read because the final state of the buttons don't depend on
the call order of the involved functions anymore. Instead there is only
one function now which enables and disables them.

This also fixes one TODO in the multithreading patch.
@MKleusberg

This comment has been minimized.

Copy link
Member Author

MKleusberg commented Jun 7, 2018

I have rewritten the commit series and rebased this branch to master. Also I have fixed the first TODO from the list above.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jun 7, 2018

Whoops, was doing stuff offline so didn't see this. I'll try it on windows in a bit. 😄

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jun 7, 2018

It's compiling now. Some more warnings we might want to clean up too:

6>C:\git_repos\sqlitebrowser\src\ImportCsvDialog.cpp(263): warning C4267: 'argument' : conversion from 'size_t' to 'int', possible loss of data
6>C:\git_repos\sqlitebrowser\src\ImportCsvDialog.cpp(273): warning C4267: 'argument' : conversion from 'size_t' to 'int', possible loss of data
6>C:\git_repos\sqlitebrowser\src\ImportCsvDialog.cpp(425): warning C4267: 'argument' : conversion from 'size_t' to 'int', possible loss of data
6>C:\git_repos\sqlitebrowser\src\ImportCsvDialog.cpp(448): warning C4267: 'argument' : conversion from 'size_t' to 'int', possible loss of data
6>C:\git_repos\sqlitebrowser\src\ImportCsvDialog.cpp(641): warning C4267: 'argument' : conversion from 'size_t' to 'int', possible loss of data
6>C:\git_repos\sqlitebrowser\src\ImportCsvDialog.cpp(643): warning C4267: 'argument' : conversion from 'size_t' to 'int', possible loss of data
6>C:\git_repos\sqlitebrowser\src\ImportCsvDialog.cpp(649): warning C4267: 'argument' : conversion from 'size_t' to 'int', possible loss of data
6>C:\git_repos\sqlitebrowser\src\sqlitedb.cpp(581): warning C4267: 'argument' : conversion from 'size_t' to 'int', possible loss of data
6>C:\git_repos\sqlitebrowser\src\sqlitedb.cpp(681): warning C4267: 'argument' : conversion from 'size_t' to 'int', possible loss of data
6>C:\git_repos\sqlitebrowser\src\sqlitedb.cpp(827): warning C4267: 'argument' : conversion from 'size_t' to 'int', possible loss of data
6>C:\git_repos\sqlitebrowser\src\sqlitedb.cpp(844): warning C4267: 'argument' : conversion from 'size_t' to 'int', possible loss of data
6>  sqlitetablemodel.cpp
6>C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(194): warning C4267: 'argument' : conversion from 'size_t' to 'int', possible loss of data
6>C:\dev\Qt\5.7\msvc2013_64\include\QtCore/qobjectdefs_impl.h(205): warning C4267: 'argument' : conversion from 'size_t' to 'const unsigned int', possible loss of data
6>          C:\dev\Qt\5.7\msvc2013_64\include\QtCore/qobjectdefs_impl.h(220) : see reference to class template instantiation 'QtPrivate::AreArgumentsCompatible<unsigned __int64,unsigned int>' being compiled
6>          C:\dev\Qt\5.7\msvc2013_64\include\QtCore/qobjectdefs_impl.h(221) : see reference to class template instantiation 'QtPrivate::CheckCompatibleArguments<QtPrivate::List<size_t,size_t>,QtPrivate::List<unsigned int,unsigned int>>' being compiled
6>          C:\dev\Qt\5.7\msvc2013_64\include\QtCore/qobject.h(232) : see reference to class template instantiation 'QtPrivate::CheckCompatibleArguments<QtPrivate::List<int,size_t,size_t>,QtPrivate::List<int,unsigned int,unsigned int>>' being compiled
6>          C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(34) : see reference to function template instantiation 'QMetaObject::Connection QObject::connect<void(__cdecl RowLoader::* )(int,size_t,size_t),void(__cdecl SqliteTableModel::* )(int,unsigned int,unsigned int)>(const RowLoader *,Func1,const SqliteTableModel *,Func2,Qt::ConnectionType)' being compiled
6>          with
6>          [
6>              Func1=void (__cdecl RowLoader::* )(int,size_t,size_t)
6>  ,            Func2=void (__cdecl SqliteTableModel::* )(int,unsigned int,unsigned int)
6>          ]
6>C:\dev\Qt\5.7\msvc2013_64\include\QtCore/qobjectdefs_impl.h(141): warning C4267: 'argument' : conversion from 'size_t' to 'unsigned int', possible loss of data
6>          C:\dev\Qt\5.7\msvc2013_64\include\QtCore/qobjectdefs_impl.h(140) : while compiling class template member function 'void QtPrivate::FunctorCall<QtPrivate::IndexesList<0,1,2>,SignalArgs,R,void (__cdecl SqliteTableModel::* )(int,unsigned int,unsigned int)>::call(SlotRet (__cdecl SqliteTableModel::* )(int,unsigned int,unsigned int),Obj *,void **)'
6>          with
6>          [
6>              SignalArgs=QtPrivate::List<int,size_t,size_t>
6>  ,            R=void
6>  ,            SlotRet=void
6>  ,            Obj=SqliteTableModel
6>          ]
6>          C:\dev\Qt\5.7\msvc2013_64\include\QtCore/qobjectdefs_impl.h(160) : see reference to function template instantiation 'void QtPrivate::FunctorCall<QtPrivate::IndexesList<0,1,2>,SignalArgs,R,void (__cdecl SqliteTableModel::* )(int,unsigned int,unsigned int)>::call(SlotRet (__cdecl SqliteTableModel::* )(int,unsigned int,unsigned int),Obj *,void **)' being compiled
6>          with
6>          [
6>              SignalArgs=QtPrivate::List<int,size_t,size_t>
6>  ,            R=void
6>  ,            SlotRet=void
6>  ,            Obj=SqliteTableModel
6>          ]
6>          C:\dev\Qt\5.7\msvc2013_64\include\QtCore/qobjectdefs_impl.h(160) : see reference to class template instantiation 'QtPrivate::FunctorCall<QtPrivate::IndexesList<0,1,2>,SignalArgs,R,void (__cdecl SqliteTableModel::* )(int,unsigned int,unsigned int)>' being compiled
6>          with
6>          [
6>              SignalArgs=QtPrivate::List<int,size_t,size_t>
6>  ,            R=void
6>          ]
6>          C:\dev\Qt\5.7\msvc2013_64\include\QtCore/qobject_impl.h(120) : see reference to function template instantiation 'void QtPrivate::FunctionPointer<void (__cdecl SqliteTableModel::* )(int,unsigned int,unsigned int)>::call<Args,R>(void (__cdecl SqliteTableModel::* )(int,unsigned int,unsigned int),Obj *,void **)' being compiled
6>          with
6>          [
6>              Args=QtPrivate::List<int,size_t,size_t>
6>  ,            R=void
6>  ,            Obj=SqliteTableModel
6>          ]
6>          C:\dev\Qt\5.7\msvc2013_64\include\QtCore/qobject_impl.h(120) : see reference to function template instantiation 'void QtPrivate::FunctionPointer<void (__cdecl SqliteTableModel::* )(int,unsigned int,unsigned int)>::call<Args,R>(void (__cdecl SqliteTableModel::* )(int,unsigned int,unsigned int),Obj *,void **)' being compiled
6>          with
6>          [
6>              Args=QtPrivate::List<int,size_t,size_t>
6>  ,            R=void
6>  ,            Obj=SqliteTableModel
6>          ]
6>          C:\dev\Qt\5.7\msvc2013_64\include\QtCore/qobject_impl.h(114) : while compiling class template member function 'void QtPrivate::QSlotObject<Func2,QtPrivate::List<int,size_t,size_t>,void>::impl(int,QtPrivate::QSlotObjectBase *,QObject *,void **,bool *)'
6>          with
6>          [
6>              Func2=void (__cdecl SqliteTableModel::* )(int,unsigned int,unsigned int)
6>          ]
6>          C:\dev\Qt\5.7\msvc2013_64\include\QtCore/qobject_impl.h(129) : see reference to function template instantiation 'void QtPrivate::QSlotObject<Func2,QtPrivate::List<int,size_t,size_t>,void>::impl(int,QtPrivate::QSlotObjectBase *,QObject *,void **,bool *)' being compiled
6>          with
6>          [
6>              Func2=void (__cdecl SqliteTableModel::* )(int,unsigned int,unsigned int)
6>          ]
6>          C:\dev\Qt\5.7\msvc2013_64\include\QtCore/qobject.h(244) : see reference to class template instantiation 'QtPrivate::QSlotObject<Func2,QtPrivate::List<int,size_t,size_t>,void>' being compiled
6>          with
6>          [
6>              Func2=void (__cdecl SqliteTableModel::* )(int,unsigned int,unsigned int)
6>          ]
6>          C:\git_repos\sqlitebrowser\src\sqlitetablemodel.cpp(34) : see reference to function template instantiation 'QMetaObject::Connection QObject::connect<void(__cdecl RowLoader::* )(int,size_t,size_t),void(__cdecl SqliteTableModel::* )(int,unsigned int,unsigned int)>(const RowLoader *,Func1,const SqliteTableModel *,Func2,Qt::ConnectionType)' being compiled
6>          with
6>          [
6>              Func1=void (__cdecl RowLoader::* )(int,size_t,size_t)
6>  ,            Func2=void (__cdecl SqliteTableModel::* )(int,unsigned int,unsigned int)
6>          ]
6>  RowLoader.cpp
6>  sqlitetypes.cpp
6>C:\git_repos\sqlitebrowser\libs\antlr-2.7.7\antlr/CharScanner.hpp(566): warning C4996: 'stricmp': The POSIX name for this item is deprecated. Instead, use the ISO C++ conformant name: _stricmp. See online help for details.
6>          C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\include\string.h(253) : see declaration of 'stricmp'
6>C:\git_repos\sqlitebrowser\libs\antlr-2.7.7\antlr/CharScanner.hpp(566): warning C4996: 'stricmp': The POSIX name for this item is deprecated. Instead, use the ISO C++ conformant name: _stricmp. See online help for details.
6>          C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\include\string.h(253) : see declaration of 'stricmp'
6>  Sqlite3Parser.cpp
6>C:\git_repos\sqlitebrowser\src\grammar\Sqlite3Parser.cpp(3926): warning C4101: 'pe' : unreferenced local variable

The build itself succeeded and seems to run ok, which is the important thing. 😄

@MKleusberg

This comment has been minimized.

Copy link
Member Author

MKleusberg commented Jun 8, 2018

The warnings mostly seem to be unrelated to the multithreading patch. So I'll probably ignore them for now to not delay this PR. But feel free to open an issue for them - always good to have no warnings on all compilers 😄

Improve loading of all data
The multithreading patch didn't properly load all data into the cache
when this was necessary. It would only do so if the chunk size was
sufficiently high. This is fixed in this commit.

Show a progress dialog while loading all data which can be cancelled by
the user.

When cancelling the loading of all data in those cases which require all
data to be loaded, stop whatever process needs the data too.
@MKleusberg

This comment has been minimized.

Copy link
Member Author

MKleusberg commented Jun 8, 2018

... and the second TODO should be fixed now 😄 Since the third one (In RowCache.h: introduce maximum segment size?) is an open question and the forth one (In SqliteTableModel::handleFinishedFetch(): optimize) is no blocker, I will merge this PR in the evening - that is, if nobody finds any other issues until then 😉

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jun 8, 2018

Sounds good to me. 😄

@MKleusberg

This comment has been minimized.

Copy link
Member Author

MKleusberg commented Jun 8, 2018

OK, going to merge this then. I'm trying to not delay this PR any further (now that the most prominent remaining problems are fixed) because I think in the nightly builds the code should get a decent amount of testing. And it can't hurt to test this a bit more before the release 😄 So if anybody notices anything (really anything; e.g. the last commit also fixed some problems which were introduced in the Edit Table dialog) please open an issue.

@MKleusberg MKleusberg merged commit 28b8652 into master Jun 8, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@MKleusberg MKleusberg deleted the multithreading branch Jun 8, 2018

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jun 9, 2018

The warnings mostly seem to be unrelated to the multithreading patch. So I'll probably ignore them for now to not delay this PR. But feel free to open an issue for them ...

Almost forgot about this. #1423 is the new issue for it, so we don't actually forget. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment