From 50cd56072276d1c0a51c11bc60bef4d756326031 Mon Sep 17 00:00:00 2001 From: uykusuz Date: Thu, 4 Jan 2018 15:15:02 +0100 Subject: [PATCH] #0: various fixes for windows build (#84) * #0: various fixes for windows build * remove warnings that definition of a variable hides previously defined ones * remove warnings that function argument is not used * remove warnings that statements are not reached * event_trace_win: fix compilation with UNICODE * #0 various windows fixes: react to PR comments * localize unicode handling of process info to ProcessInfo * fix compilation error in Properties constructor * event_trace_win: fix bug in ProcessInfo initialization We didn't retry setting the name when GetModuleBaseName() didn't succeed, even though we did that before the last refactoring. * Revert "event_trace_win: fix bug in ProcessInfo initialization" This reverts commit c4ce6cf488be5489fa0a1a9c24e92dd39bbc42a1. * Revert "#0 various windows fixes: react to PR comments" This reverts commit cae31a25371e53a3645ddab82372c3772138d658. * #0 various windows fixes: react to PR comments again * keep number of allocations minimal * add getProcessName() to retrieve the process name as char always (i.e. not UNICODE) --- easy_profiler_core/block.cpp | 6 ++- easy_profiler_core/easy_socket.cpp | 13 +++--- easy_profiler_core/event_trace_win.cpp | 50 +++++++++++++++++++--- easy_profiler_core/event_trace_win.h | 2 + easy_profiler_core/profile_manager.cpp | 3 +- easy_profiler_core/reader.cpp | 54 ++++++++++++------------ profiler_gui/blocks_tree_widget.cpp | 2 +- profiler_gui/common_types.h | 8 ---- profiler_gui/descriptors_tree_widget.cpp | 6 +-- profiler_gui/easy_graphics_scrollbar.cpp | 6 +-- profiler_gui/main_window.cpp | 4 +- profiler_gui/tree_widget_item.cpp | 2 - 12 files changed, 94 insertions(+), 62 deletions(-) diff --git a/easy_profiler_core/block.cpp b/easy_profiler_core/block.cpp index c399913..3344193 100644 --- a/easy_profiler_core/block.cpp +++ b/easy_profiler_core/block.cpp @@ -51,7 +51,7 @@ #include "profile_manager.h" #include "current_time.h" -using namespace profiler; +namespace profiler { #ifndef EASY_PROFILER_API_DISABLED Event::Event(timestamp_t _begin_time) : m_begin(_begin_time), m_end(0) @@ -230,7 +230,9 @@ CSwitchEvent::CSwitchEvent(timestamp_t _begin_time, thread_id_t _tid) } -CSwitchBlock::CSwitchBlock(timestamp_t _begin_time, thread_id_t _tid, const char* _runtimeName) +} // END of namespace profiler. + +CSwitchBlock::CSwitchBlock(profiler::timestamp_t _begin_time, profiler::thread_id_t _tid, const char* _runtimeName) : CSwitchEvent(_begin_time, _tid) , m_name(_runtimeName) { diff --git a/easy_profiler_core/easy_socket.cpp b/easy_profiler_core/easy_socket.cpp index 05f6308..ab2f5eb 100644 --- a/easy_profiler_core/easy_socket.cpp +++ b/easy_profiler_core/easy_socket.cpp @@ -106,13 +106,12 @@ int EasySocket::bind(uint16_t port) if (!checkSocket(m_socket)) return -1; - struct sockaddr_in serv_addr; - memset(&serv_addr, 0, sizeof(serv_addr)); - serv_addr.sin_family = AF_INET; - serv_addr.sin_addr.s_addr = INADDR_ANY; - serv_addr.sin_port = htons(port); - auto res = ::bind(m_socket, (struct sockaddr*)&serv_addr, sizeof(serv_addr)); - + struct sockaddr_in server_address; + memset(&server_address, 0, sizeof(server_address)); + server_address.sin_family = AF_INET; + server_address.sin_addr.s_addr = INADDR_ANY; + server_address.sin_port = htons(port); + auto res = ::bind(m_socket, (struct sockaddr *)&server_address, sizeof(server_address)); return res; } diff --git a/easy_profiler_core/event_trace_win.cpp b/easy_profiler_core/event_trace_win.cpp index c6714c6..b664f6a 100644 --- a/easy_profiler_core/event_trace_win.cpp +++ b/easy_profiler_core/event_trace_win.cpp @@ -135,6 +135,32 @@ char KERNEL_LOGGER[] = KERNEL_LOGGER_NAME; ::std::atomic_uint64_t TRACING_END_TIME = ATOMIC_VAR_INIT(~0ULL); #endif +namespace { + /** + * Retrieve the process name of the given process. + * + * This method is NOT thread-safe: the returned string has to be copied somewhere before this + * method can be used by another thread or call. + * + * getProcessName() owns the returned string. + * + * \return a pair of the process name string and the string length. + */ + std::pair getProcessName(HANDLE hProcess) + { + static TCHAR buf[MAX_PATH] = {}; + std::size_t len = static_cast(GetModuleBaseName(hProcess, 0, buf, MAX_PATH)); + +#if UNICODE + static char charbuf[MAX_PATH] = {}; + std::size_t charbufLength = std::wcstombs(charbuf, buf, len); + return std::make_pair(charbuf, charbufLength); +#else + return std::make_pair(buf, len); +#endif + } +} + namespace profiler { const decltype(EVENT_DESCRIPTOR::Opcode) SWITCH_CONTEXT_OPCODE = 36; @@ -237,14 +263,13 @@ namespace profiler { auto hProc = OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_VM_READ, FALSE, pid); if (hProc != nullptr) { - static TCHAR buf[MAX_PATH] = {}; // Using static is safe because processTraceEvent() is called from one thread - auto len = GetModuleBaseName(hProc, 0, buf, MAX_PATH); + const auto& processName = getProcessName(hProc); // Using thread-unsafe method is safe because processTraceEvent() is called from one thread - if (len != 0) + if (processName.second != 0) { - pinfo->name.reserve(pinfo->name.size() + 2 + len); + pinfo->name.reserve(pinfo->name.size() + 2 + processName.second); pinfo->name.append(" ", 1); - pinfo->name.append(buf, len); + pinfo->name.append(processName.first, processName.second); pinfo->valid = 1; } @@ -289,6 +314,17 @@ namespace profiler { MANAGER.endContextSwitch(_contextSwitchEvent->NewThreadId, pid, time); } + ////////////////////////////////////////////////////////////////////////// + + EasyEventTracer::Properties::Properties() + { +#if UNICODE + std::wcstombs(sessionName, KERNEL_LOGGER_NAME, sizeof(sessionName)); +#else + std::strncpy(sessionName, KERNEL_LOGGER_NAME, sizeof(sessionName)); +#endif + } + ////////////////////////////////////////////////////////////////////////// #ifndef EASY_MAGIC_STATIC_CPP11 @@ -328,7 +364,7 @@ namespace profiler { m_lowPriority.store(_value, ::std::memory_order_release); } - bool setPrivilege(HANDLE hToken, LPCSTR _privelegeName) + bool setPrivilege(HANDLE hToken, PTCHAR _privelegeName) { bool success = false; @@ -409,7 +445,7 @@ namespace profiler { */ // static is safe because we are guarded by spin-lock m_spin - static Properties p = ([]{ Properties prp; strncpy(prp.sessionName, KERNEL_LOGGER_NAME, sizeof(prp.sessionName)); return prp; })(); + static Properties p; p.base = m_properties.base; // Use copy of m_properties to make sure m_properties will not be changed // Stop another session diff --git a/easy_profiler_core/event_trace_win.h b/easy_profiler_core/event_trace_win.h index 7c112de..d502844 100644 --- a/easy_profiler_core/event_trace_win.h +++ b/easy_profiler_core/event_trace_win.h @@ -81,6 +81,8 @@ namespace profiler { #pragma pack(push, 1) struct Properties { + Properties(); + EVENT_TRACE_PROPERTIES base; char sessionName[sizeof(KERNEL_LOGGER_NAME)]; }; diff --git a/easy_profiler_core/profile_manager.cpp b/easy_profiler_core/profile_manager.cpp index e069bb0..5dc4022 100644 --- a/easy_profiler_core/profile_manager.cpp +++ b/easy_profiler_core/profile_manager.cpp @@ -763,6 +763,7 @@ const BaseBlockDescriptor* ProfileManager::addBlockDescriptor(EasyBlockStatus _d } #else auto desc = new BlockDescriptor(static_cast(m_descriptors.size()), _defaultStatus, _name, _filename, _line, _block_type, _color); + (void)_copyName; // unused #endif m_descriptors.emplace_back(desc); @@ -1178,7 +1179,7 @@ void ProfileManager::setEnabled(bool isEnable) guard_lock_t lock(m_dumpSpin); auto time = getCurrentTime(); - const auto status = isEnable ? EASY_PROF_ENABLED : EASY_PROF_DISABLED; + const auto status = static_cast(isEnable ? EASY_PROF_ENABLED : EASY_PROF_DISABLED); const auto prev = m_profilerStatus.exchange(status, std::memory_order_release); if (prev == status) return; diff --git a/easy_profiler_core/reader.cpp b/easy_profiler_core/reader.cpp index d7e696e..7dd6e1e 100644 --- a/easy_profiler_core/reader.cpp +++ b/easy_profiler_core/reader.cpp @@ -374,6 +374,20 @@ void validate_pointers(::std::atomic& _progress, const char* _oldbase, ::pr ////////////////////////////////////////////////////////////////////////// +static bool update_progress(::std::atomic& progress, int new_value, ::std::stringstream& _log) +{ + auto oldprogress = progress.exchange(new_value, ::std::memory_order_release); + if (oldprogress < 0) + { + _log << "Reading was interrupted"; + return false; + } + + return true; +} + +////////////////////////////////////////////////////////////////////////// + extern "C" { PROFILER_API ::profiler::block_index_t fillTreesFromFile(::std::atomic& progress, const char* filename, @@ -387,10 +401,8 @@ extern "C" { bool gather_statistics, ::std::stringstream& _log) { - auto oldprogress = progress.exchange(0, ::std::memory_order_release); - if (oldprogress < 0) + if (!update_progress(progress, 0, _log)) { - _log << "Reading was interrupted"; return 0; } @@ -433,10 +445,8 @@ extern "C" { { EASY_FUNCTION(::profiler::colors::Cyan); - auto oldprogress = progress.exchange(0, ::std::memory_order_release); - if (oldprogress < 0) + if (!update_progress(progress, 0, _log)) { - _log << "Reading was interrupted"; return 0; } @@ -545,11 +555,9 @@ extern "C" { descriptors.push_back(descriptor); i += sz; - auto oldprogress = progress.exchange(static_cast(15 * i / descriptors_memory_size), ::std::memory_order_release); - if (oldprogress < 0) + if (!update_progress(progress, static_cast(15 * i / descriptors_memory_size), _log)) { - _log << "Reading was interrupted"; - return 0; // Loading interrupted + return 0; } } @@ -641,10 +649,8 @@ extern "C" { } } - auto oldprogress = progress.exchange(20 + static_cast(70 * i / memory_size), ::std::memory_order_release); - if (oldprogress < 0) + if (!update_progress(progress, 20 + static_cast(70 * i / memory_size), _log)) { - _log << "Reading was interrupted"; return 0; // Loading interrupted } } @@ -759,19 +765,19 @@ extern "C" { //per_parent_statistics.reserve(tree.children.size() * 2); // this gives no speed-up on Windows // TODO: check this behavior on Linux - for (auto i : tree.children) + for (auto child_block_index : tree.children) { - auto& child = blocks[i]; - child.per_parent_stats = update_statistics(per_parent_statistics, child, i, block_index, blocks); + auto& child = blocks[child_block_index]; + child.per_parent_stats = update_statistics(per_parent_statistics, child, child_block_index, block_index, blocks); if (tree.depth < child.depth) tree.depth = child.depth; } } else { - for (auto i : tree.children) + for (auto child_block_index : tree.children) { - const auto& child = blocks[i]; + const auto& child = blocks[child_block_index]; if (tree.depth < child.depth) tree.depth = child.depth; } @@ -807,10 +813,8 @@ extern "C" { } } - auto oldprogress = progress.exchange(20 + static_cast(70 * i / memory_size), ::std::memory_order_release); - if (oldprogress < 0) + if (!update_progress(progress, 20 + static_cast(70 * i / memory_size), _log)) { - _log << "Reading was interrupted"; return 0; // Loading interrupted } } @@ -905,9 +909,9 @@ extern "C" { //}); //root.tree.shrink_to_fit(); - for (auto i : root.children) + for (auto child_block_index : root.children) { - auto& frame = blocks[i]; + auto& frame = blocks[child_block_index]; if (descriptors[frame.node->id()]->type() == ::profiler::BLOCK_TYPE_BLOCK) ++root.frames_number; @@ -998,10 +1002,8 @@ extern "C" { descriptors.push_back(descriptor); i += sz; - auto oldprogress = progress.exchange(static_cast(100 * i / descriptors_memory_size), ::std::memory_order_release); - if (oldprogress < 0) + if (!update_progress(progress, static_cast(100 * i / descriptors_memory_size), _log)) { - _log << "Reading was interrupted"; return false; // Loading interrupted } } diff --git a/profiler_gui/blocks_tree_widget.cpp b/profiler_gui/blocks_tree_widget.cpp index f3ab80b..abb5a18 100644 --- a/profiler_gui/blocks_tree_widget.cpp +++ b/profiler_gui/blocks_tree_widget.cpp @@ -795,7 +795,7 @@ void EasyTreeWidget::onCollapseAllClicked(bool) void EasyTreeWidget::onExpandAllClicked(bool) { - const QSignalBlocker b(this); + const QSignalBlocker blocker(this); m_bSilentExpandCollapse = true; expandAll(); diff --git a/profiler_gui/common_types.h b/profiler_gui/common_types.h index 3e89db3..ebee20f 100644 --- a/profiler_gui/common_types.h +++ b/profiler_gui/common_types.h @@ -328,8 +328,6 @@ inline QString timeStringReal(TimeUnits _units, qreal _interval, int _precision default: return autoTimeStringReal(_interval, _precision); } - - return QString(); } inline QString timeStringRealNs(TimeUnits _units, ::profiler::timestamp_t _interval, int _precision = 1) @@ -351,8 +349,6 @@ inline QString timeStringRealNs(TimeUnits _units, ::profiler::timestamp_t _inter default: return autoTimeStringRealNs(_interval, _precision); } - - return QString(); } inline QString timeStringInt(TimeUnits _units, qreal _interval) @@ -372,8 +368,6 @@ inline QString timeStringInt(TimeUnits _units, qreal _interval) default: return autoTimeStringInt(_interval); } - - return QString(); } inline QString timeStringIntNs(TimeUnits _units, ::profiler::timestamp_t _interval) @@ -393,8 +387,6 @@ inline QString timeStringIntNs(TimeUnits _units, ::profiler::timestamp_t _interv default: return autoTimeStringIntNs(_interval); } - - return QString(); } ////////////////////////////////////////////////////////////////////////// diff --git a/profiler_gui/descriptors_tree_widget.cpp b/profiler_gui/descriptors_tree_widget.cpp index 0bfa09f..10e2ab9 100644 --- a/profiler_gui/descriptors_tree_widget.cpp +++ b/profiler_gui/descriptors_tree_widget.cpp @@ -269,14 +269,14 @@ void EasyDescTreeWidget::contextMenuEvent(QContextMenuEvent* _event) connect(action, &QAction::triggered, this, &This::collapseAll); menu.addSeparator(); - auto submenu = menu.addMenu("Search by"); + auto searchby_submenu = menu.addMenu("Search by"); auto header_item = headerItem(); for (int i = 0; i < DESC_COL_STATUS; ++i) { if (i == DESC_COL_TYPE) continue; - action = submenu->addAction(header_item->text(i)); + action = searchby_submenu->addAction(header_item->text(i)); action->setData(i); action->setCheckable(true); if (i == m_searchColumn) @@ -537,7 +537,7 @@ void EasyDescTreeWidget::onBlockStatusChangeClicked(bool _checked) } } -void EasyDescTreeWidget::onBlockStatusChange(::profiler::block_id_t _id, ::profiler::EasyBlockStatus _status) +void EasyDescTreeWidget::onBlockStatusChange(::profiler::block_id_t _id, ::profiler::EasyBlockStatus /* _status */) { if (m_bLocked) return; diff --git a/profiler_gui/easy_graphics_scrollbar.cpp b/profiler_gui/easy_graphics_scrollbar.cpp index a8de2e0..97beb0c 100644 --- a/profiler_gui/easy_graphics_scrollbar.cpp +++ b/profiler_gui/easy_graphics_scrollbar.cpp @@ -154,7 +154,7 @@ EasyGraphicsSliderItem::~EasyGraphicsSliderItem() } -void EasyGraphicsSliderItem::paint(QPainter* _painter, const QStyleOptionGraphicsItem* _option, QWidget* _widget) +void EasyGraphicsSliderItem::paint(QPainter* _painter, const QStyleOptionGraphicsItem* /* _option */, QWidget* /* _widget */) { if (static_cast(scene()->parent())->bindMode()) { @@ -280,7 +280,7 @@ QRectF EasyHistogramItem::boundingRect() const return m_boundingRect; } -void EasyHistogramItem::paint(QPainter* _painter, const QStyleOptionGraphicsItem* _option, QWidget* _widget) +void EasyHistogramItem::paint(QPainter* _painter, const QStyleOptionGraphicsItem* /* _option */, QWidget* /* _widget */) { if (!m_bPermitImageUpdate || (m_regime == Hist_Pointer && m_pSource == nullptr) || (m_regime == Hist_Id && (m_threadId == 0 || ::profiler_gui::is_max(m_blockId)))) return; @@ -1401,7 +1401,7 @@ void EasyHistogramItem::updateImage(QRectF _boundingRect, HistRegime _regime, qr qreal _minimum, qreal _maximum, qreal _range, qreal _value, qreal _width, qreal _top_duration, qreal _bottom_duration, bool _bindMode, float _frame_time, ::profiler::timestamp_t _begin_time, - qreal _origin, bool _autoAdjustHist) + qreal /* _origin */, bool _autoAdjustHist) { const auto bottom = _boundingRect.height();//_boundingRect.bottom(); const auto screenWidth = _boundingRect.width() * _current_scale; diff --git a/profiler_gui/main_window.cpp b/profiler_gui/main_window.cpp index 984018e..8a58352 100644 --- a/profiler_gui/main_window.cpp +++ b/profiler_gui/main_window.cpp @@ -783,14 +783,14 @@ void EasyMainWindow::loadFile(const QString& filename) m_reader.load(filename); } -void EasyMainWindow::readStream(::std::stringstream& data) +void EasyMainWindow::readStream(::std::stringstream& _data) { m_progress->setLabelText(tr("Reading from stream...")); m_progress->setValue(0); m_progress->show(); m_readerTimer.start(LOADER_TIMER_INTERVAL); - m_reader.load(data); + m_reader.load(_data); } ////////////////////////////////////////////////////////////////////////// diff --git a/profiler_gui/tree_widget_item.cpp b/profiler_gui/tree_widget_item.cpp index a955f96..b54bca0 100644 --- a/profiler_gui/tree_widget_item.cpp +++ b/profiler_gui/tree_widget_item.cpp @@ -113,8 +113,6 @@ bool EasyTreeWidgetItem::operator < (const Parent& _other) const return data(col, Qt::UserRole).toULongLong() < _other.data(col, Qt::UserRole).toULongLong(); } } - - return false; } ::profiler::block_index_t EasyTreeWidgetItem::block_index() const