From 37795cd5965eff0344589aba00b0447e8ca91d36 Mon Sep 17 00:00:00 2001 From: rdb Date: Sat, 26 Nov 2022 16:52:37 +0100 Subject: [PATCH 01/14] rplight: Fix redundant `get_lens()` calls --- contrib/src/rplight/pssmCameraRig.cxx | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/contrib/src/rplight/pssmCameraRig.cxx b/contrib/src/rplight/pssmCameraRig.cxx index f9744e2129..7bbbd0df6e 100644 --- a/contrib/src/rplight/pssmCameraRig.cxx +++ b/contrib/src/rplight/pssmCameraRig.cxx @@ -296,9 +296,10 @@ void PSSMCameraRig::compute_pssm_splits(const LMatrix4& transform, float max_dis // Reset the film size, offset and far-plane Camera* cam = DCAST(Camera, _cam_nodes[i].node()); - cam->get_lens()->set_film_size(1, 1); - cam->get_lens()->set_film_offset(0, 0); - cam->get_lens()->set_near_far(1, 100); + Lens *lens = cam->get_lens(); + lens->set_film_size(1, 1); + lens->set_film_offset(0, 0); + lens->set_near_far(1, 100); // Find a good initial position _cam_nodes[i].set_pos(cam_start); @@ -320,16 +321,16 @@ void PSSMCameraRig::compute_pssm_splits(const LMatrix4& transform, float max_dis if (_max_film_sizes[i].get_x() < film_size.get_x()) _max_film_sizes[i].set_x(film_size.get_x()); if (_max_film_sizes[i].get_y() < film_size.get_y()) _max_film_sizes[i].set_y(film_size.get_y()); - cam->get_lens()->set_film_size(_max_film_sizes[i] * filmsize_bias); + lens->set_film_size(_max_film_sizes[i] * filmsize_bias); } else { // If we don't use a fixed film size, we can just set the film size // on the lens. - cam->get_lens()->set_film_size(film_size * filmsize_bias); + lens->set_film_size(film_size * filmsize_bias); } // Compute new film offset - cam->get_lens()->set_film_offset(film_offset); - cam->get_lens()->set_near_far(10, best_max_extent.get_z()); + lens->set_film_offset(film_offset); + lens->set_near_far(10, best_max_extent.get_z()); _camera_nearfar[i] = LVecBase2(10, best_max_extent.get_z()); // Compute the camera MVP From 5db4b447a6a9270150678a2a053125696512a3ac Mon Sep 17 00:00:00 2001 From: rdb Date: Sat, 26 Nov 2022 16:52:55 +0100 Subject: [PATCH 02/14] rplight: Fix PSSM calculation failing with infinite far distance Fixes #1397 --- contrib/src/rplight/pssmCameraRig.cxx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contrib/src/rplight/pssmCameraRig.cxx b/contrib/src/rplight/pssmCameraRig.cxx index 7bbbd0df6e..410ded4c83 100644 --- a/contrib/src/rplight/pssmCameraRig.cxx +++ b/contrib/src/rplight/pssmCameraRig.cxx @@ -400,7 +400,8 @@ void PSSMCameraRig::update(NodePath cam_node, const LVecBase3 &light_vector) { } // Do the actual PSSM - compute_pssm_splits( transform, _pssm_distance / lens->get_far(), light_vector ); + double far_recip = std::max(1.0 / (double)lens->get_far(), (double)lens_far_limit); + compute_pssm_splits( transform, _pssm_distance * far_recip, light_vector ); _update_collector.stop(); } From d00d4a44a070971b5c5d9a6d02a2c526313fd967 Mon Sep 17 00:00:00 2001 From: rdb Date: Sat, 26 Nov 2022 16:56:07 +0100 Subject: [PATCH 03/14] gobj: Properly initialize Texture object on all stages Previously, forward stages would be uninitialized, resulting in cryptic errors with ahead-of-time preparation (although arguably ahead-of-time preparation is a bug in and of itself) --- panda/src/gobj/texture.cxx | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/panda/src/gobj/texture.cxx b/panda/src/gobj/texture.cxx index 318eaf59c9..703b70c4ae 100644 --- a/panda/src/gobj/texture.cxx +++ b/panda/src/gobj/texture.cxx @@ -380,8 +380,7 @@ Texture(const string &name) : _reloading = false; CDWriter cdata(_cycler, true); - do_set_format(cdata, F_rgb); - do_set_component_type(cdata, T_unsigned_byte); + cdata->inc_properties_modified(); } /** @@ -10677,11 +10676,10 @@ CData() { _y_size = 1; _z_size = 1; _num_views = 1; - - // We will override the format in a moment (in the Texture constructor), but - // set it to something else first to avoid the check in do_set_format - // depending on an uninitialized value. - _format = F_rgba; + _num_components = 3; + _component_width = 1; + _format = F_rgb; + _component_type = T_unsigned_byte; // Only used for buffer textures. _usage_hint = GeomEnums::UH_unspecified; From 52474465006877516fb8eb88e5c5dddba681b1d4 Mon Sep 17 00:00:00 2001 From: rdb Date: Sat, 26 Nov 2022 19:23:36 +0100 Subject: [PATCH 04/14] pstats: Performance improvements for time-based strip charts Remove the use of set and list, which are allocator-heavy and insertion was a bottleneck. Since each sample occurs only once on the linked list, we can more efficiently roll our own linked list with next and prev pointers, so no allocation needed. Instead of the set, we can just store a per-collector flag. --- pandatool/src/pstatserver/pStatView.cxx | 121 ++++++++++++------------ 1 file changed, 58 insertions(+), 63 deletions(-) diff --git a/pandatool/src/pstatserver/pStatView.cxx b/pandatool/src/pstatserver/pStatView.cxx index e69959a731..09c9d95c62 100644 --- a/pandatool/src/pstatserver/pStatView.cxx +++ b/pandatool/src/pstatserver/pStatView.cxx @@ -15,13 +15,9 @@ #include "pStatFrameData.h" #include "pStatCollectorDef.h" -#include "vector_int.h" -#include "plist.h" -#include "pset.h" #include - - +#include /** * This class is used within this module only--in fact, within @@ -30,15 +26,7 @@ */ class FrameSample { public: - typedef plist Started; - - FrameSample() { - _touched = false; - _is_started = false; - _pushed = false; - _net_time = 0.0; - } - void data_point(double time, bool is_start, Started &started) { + void data_point(double time, bool is_start, FrameSample *started) { _touched = true; // We only consider events that change the startstop state. With two @@ -58,20 +46,25 @@ public: if (_pushed) { nassertv(!_is_started); - Started::iterator si = find(started.begin(), started.end(), this); - nassertv(si != started.end()); - started.erase(si); - + nassertv(_next != nullptr && _prev != nullptr); + _prev->_next = _next; + _next->_prev = _prev; + _next = _prev = nullptr; } else { if (_is_started) { _net_time -= time; push_all(time, started); - started.push_back(this); + nassertv(_next == nullptr && _prev == nullptr); + _prev = started->_prev; + _next = started; + _prev->_next = this; + started->_prev = this; } else { _net_time += time; - Started::iterator si = find(started.begin(), started.end(), this); - nassertv(si != started.end()); - started.erase(si); + nassertv(_next != nullptr && _prev != nullptr); + _prev->_next = _next; + _next->_prev = _prev; + _next = _prev = nullptr; pop_one(time, started); } } @@ -93,31 +86,32 @@ public: } } - void push_all(double time, Started &started) { - Started::iterator si; - for (si = started.begin(); si != started.end(); ++si) { - (*si)->push(time); + void push_all(double time, FrameSample *started) { + for (FrameSample *sample = started->_next; + sample != started; sample = sample->_next) { + sample->push(time); } } - void pop_one(double time, Started &started) { - Started::reverse_iterator si; - for (si = started.rbegin(); si != started.rend(); ++si) { - if ((*si)->_pushed) { - (*si)->pop(time); + void pop_one(double time, FrameSample *started) { + for (FrameSample *sample = started->_prev; + sample != started; sample = sample->_prev) { + if (sample->_pushed) { + sample->pop(time); return; } } } - bool _touched; - bool _is_started; - bool _pushed; - double _net_time; + FrameSample *_next = nullptr; + FrameSample *_prev = nullptr; + bool _touched = false; + bool _is_started = false; + bool _pushed = false; + bool _is_new = false; + double _net_time = 0.0; }; - - /** * */ @@ -279,19 +273,19 @@ get_level(int collector) { void PStatView:: update_time_data(const PStatFrameData &frame_data) { int num_events = frame_data.get_num_events(); + int num_collectors = _client_data->get_num_collectors(); - typedef pvector Samples; - Samples samples(_client_data->get_num_collectors()); + typedef std::vector Samples; + Samples samples(num_collectors); - FrameSample::Started started; + // Keep a linked list of started samples. + FrameSample started; + started._next = &started; + started._prev = &started; _all_collectors_known = true; - - // This tracks the set of samples we actually care about. - typedef pset GotSamples; - GotSamples got_samples; - + int new_collectors = 0; int i; for (i = 0; i < num_events; i++) { int collector_index = frame_data.get_time_collector(i); @@ -301,7 +295,7 @@ update_time_data(const PStatFrameData &frame_data) { _all_collectors_known = false; } else { - nassertv(collector_index >= 0 && collector_index < (int)samples.size()); + nassertv(collector_index >= 0 && collector_index < num_collectors); if (_client_data->get_child_distance(_constraint, collector_index) >= 0) { // Here's a data point we care about: anything at constraint level or @@ -310,8 +304,8 @@ update_time_data(const PStatFrameData &frame_data) { if (!is_start) { // A "stop" in the middle of a frame implies a "start" since time // 0 (that is, since the first data point in the frame). - samples[collector_index].data_point(frame_data.get_time(0), true, started); - samples[collector_index].data_point(frame_data.get_time(i), is_start, started); + samples[collector_index].data_point(frame_data.get_time(0), true, &started); + samples[collector_index].data_point(frame_data.get_time(i), is_start, &started); } else { // An extra "start" for a collector that's already started is an // error. @@ -320,23 +314,25 @@ update_time_data(const PStatFrameData &frame_data) { << "\n"; } } else { - samples[collector_index].data_point(frame_data.get_time(i), is_start, started); - got_samples.insert(collector_index); + samples[collector_index].data_point(frame_data.get_time(i), is_start, &started); + if (!samples[collector_index]._is_new) { + samples[collector_index]._is_new = true; + ++new_collectors; + } } } } } // Make sure everything is stopped. - Samples::iterator si; for (i = 0, si = samples.begin(); si != samples.end(); ++i, ++si) { if ((*si)._is_started) { - (*si).data_point(frame_data.get_end(), false, started); + (*si).data_point(frame_data.get_end(), false, &started); } } - nassertv(started.empty()); + nassertv(started._next == &started && started._prev == &started); bool any_new_levels = false; @@ -356,11 +352,10 @@ update_time_data(const PStatFrameData &frame_data) { } int collector_index = level->_collector; - GotSamples::iterator gi; - gi = got_samples.find(collector_index); - if (gi != got_samples.end()) { + if (samples[collector_index]._is_new) { level->_value_alone = samples[collector_index]._net_time; - got_samples.erase(gi); + samples[collector_index]._is_new = false; + --new_collectors; } li = lnext; @@ -368,14 +363,14 @@ update_time_data(const PStatFrameData &frame_data) { // Finally, any samples left over in the got_samples set are new collectors // that we need to add to the Levels list. - if (!got_samples.empty()) { + if (new_collectors > 0) { any_new_levels = true; - GotSamples::const_iterator gi; - for (gi = got_samples.begin(); gi != got_samples.end(); ++gi) { - int collector_index = (*gi); - PStatViewLevel *level = get_level(collector_index); - level->_value_alone = samples[*gi]._net_time; + for (int collector_index = 0; collector_index < num_collectors; ++collector_index) { + if (samples[collector_index]._is_new) { + PStatViewLevel *level = get_level(collector_index); + level->_value_alone = samples[collector_index]._net_time; + } } } From 3468b95fa9bae6d9d3644d6ba9cb715607f47f49 Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 29 Nov 2022 13:01:55 +0100 Subject: [PATCH 05/14] pstats: Show collector start/stop pair count in time-based graphs It can be very useful to know how often a collector was invoked in a frame, not just how long it took. This adds a number to the upper-right corner showing exactly that (but not aggregated, just for leaf collectors). --- pandatool/src/gtk-stats/gtkStatsStripChart.cxx | 3 ++- pandatool/src/pstatserver/pStatStripChart.I | 9 +++++++++ pandatool/src/pstatserver/pStatStripChart.cxx | 16 +++++++++++----- pandatool/src/pstatserver/pStatStripChart.h | 3 ++- pandatool/src/pstatserver/pStatView.cxx | 7 +++++++ pandatool/src/pstatserver/pStatViewLevel.I | 8 ++++++++ pandatool/src/pstatserver/pStatViewLevel.h | 2 ++ pandatool/src/win-stats/winStatsStripChart.cxx | 2 +- 8 files changed, 42 insertions(+), 8 deletions(-) diff --git a/pandatool/src/gtk-stats/gtkStatsStripChart.cxx b/pandatool/src/gtk-stats/gtkStatsStripChart.cxx index 1015c7989a..094b3dd53f 100644 --- a/pandatool/src/gtk-stats/gtkStatsStripChart.cxx +++ b/pandatool/src/gtk-stats/gtkStatsStripChart.cxx @@ -15,6 +15,7 @@ #include "gtkStatsMonitor.h" #include "pStatCollectorDef.h" #include "numeric_types.h" +#include "string_utils.h" static const int default_strip_chart_width = 400; static const int default_strip_chart_height = 100; @@ -120,7 +121,7 @@ new_data(int thread_index, int frame_number) { if (!_pause) { update(); - std::string text = format_number(get_average_net_value(), get_guide_bar_units(), get_guide_bar_unit_name()); + std::string text = get_total_text(); if (_net_value_text != text) { _net_value_text = text; gtk_label_set_text(GTK_LABEL(_total_label), _net_value_text.c_str()); diff --git a/pandatool/src/pstatserver/pStatStripChart.I b/pandatool/src/pstatserver/pStatStripChart.I index 791e722009..35d4c86265 100644 --- a/pandatool/src/pstatserver/pStatStripChart.I +++ b/pandatool/src/pstatserver/pStatStripChart.I @@ -156,6 +156,15 @@ pixel_to_height(int x) const { return _value_height * (double)(get_ysize() - x) / (double)get_ysize(); } +/** + * Returns true if get_title_text() has never yet returned an answer, false if + * it has. + */ +INLINE bool PStatStripChart:: +is_title_unknown() const { + return _title_unknown; +} + /** * Returns true if the indicated collector appears anywhere on the chart at * the current time, false otherwise. diff --git a/pandatool/src/pstatserver/pStatStripChart.cxx b/pandatool/src/pstatserver/pStatStripChart.cxx index c9ffcb3109..034022499c 100644 --- a/pandatool/src/pstatserver/pStatStripChart.cxx +++ b/pandatool/src/pstatserver/pStatStripChart.cxx @@ -284,12 +284,18 @@ get_title_text() { } /** - * Returns true if get_title_text() has never yet returned an answer, false if - * it has. + * Returns the text suitable for the total label above the graph. */ -bool PStatStripChart:: -is_title_unknown() const { - return _title_unknown; +std::string PStatStripChart:: +get_total_text() { + std::string text = format_number(get_average_net_value(), get_guide_bar_units(), get_guide_bar_unit_name()); + if (get_collector_index() != 0 && !_view.get_show_level()) { + const PStatViewLevel *level = _view.get_level(get_collector_index()); + if (level != nullptr && level->get_count() > 0) { + text += " / " + format_string(level->get_count()) + "x"; + } + } + return text; } /** diff --git a/pandatool/src/pstatserver/pStatStripChart.h b/pandatool/src/pstatserver/pStatStripChart.h index 5cba0164e2..3851047ed0 100644 --- a/pandatool/src/pstatserver/pStatStripChart.h +++ b/pandatool/src/pstatserver/pStatStripChart.h @@ -68,8 +68,9 @@ public: INLINE int height_to_pixel(double value) const; INLINE double pixel_to_height(int y) const; + INLINE bool is_title_unknown() const; std::string get_title_text(); - bool is_title_unknown() const; + std::string get_total_text(); protected: class ColorData { diff --git a/pandatool/src/pstatserver/pStatView.cxx b/pandatool/src/pstatserver/pStatView.cxx index 09c9d95c62..2e255273f7 100644 --- a/pandatool/src/pstatserver/pStatView.cxx +++ b/pandatool/src/pstatserver/pStatView.cxx @@ -109,6 +109,7 @@ public: bool _is_started = false; bool _pushed = false; bool _is_new = false; + int _count = 0; double _net_time = 0.0; }; @@ -315,6 +316,9 @@ update_time_data(const PStatFrameData &frame_data) { } } else { samples[collector_index].data_point(frame_data.get_time(i), is_start, &started); + if (is_start) { + samples[collector_index]._count++; + } if (!samples[collector_index]._is_new) { samples[collector_index]._is_new = true; ++new_collectors; @@ -354,6 +358,7 @@ update_time_data(const PStatFrameData &frame_data) { int collector_index = level->_collector; if (samples[collector_index]._is_new) { level->_value_alone = samples[collector_index]._net_time; + level->_count = samples[collector_index]._count; samples[collector_index]._is_new = false; --new_collectors; } @@ -370,6 +375,7 @@ update_time_data(const PStatFrameData &frame_data) { if (samples[collector_index]._is_new) { PStatViewLevel *level = get_level(collector_index); level->_value_alone = samples[collector_index]._net_time; + level->_count = samples[collector_index]._count; } } } @@ -504,6 +510,7 @@ bool PStatView:: reset_level(PStatViewLevel *level) { bool any_changed = false; level->_value_alone = 0.0; + level->_count = 0; if (level->_collector == _constraint) { return false; diff --git a/pandatool/src/pstatserver/pStatViewLevel.I b/pandatool/src/pstatserver/pStatViewLevel.I index f682c36b73..38fdec95a6 100644 --- a/pandatool/src/pstatserver/pStatViewLevel.I +++ b/pandatool/src/pstatserver/pStatViewLevel.I @@ -27,3 +27,11 @@ INLINE double PStatViewLevel:: get_value_alone() const { return _value_alone; } + +/** + * Returns the number of start/stop pairs for this collector. + */ +INLINE int PStatViewLevel:: +get_count() const { + return _count; +} diff --git a/pandatool/src/pstatserver/pStatViewLevel.h b/pandatool/src/pstatserver/pStatViewLevel.h index 006a512ddc..802850c04b 100644 --- a/pandatool/src/pstatserver/pStatViewLevel.h +++ b/pandatool/src/pstatserver/pStatViewLevel.h @@ -31,6 +31,7 @@ public: INLINE int get_collector() const; INLINE double get_value_alone() const; double get_net_value() const; + INLINE int get_count() const; void sort_children(const PStatClientData *client_data); @@ -39,6 +40,7 @@ public: private: int _collector; + int _count = 0; double _value_alone; PStatViewLevel *_parent; diff --git a/pandatool/src/win-stats/winStatsStripChart.cxx b/pandatool/src/win-stats/winStatsStripChart.cxx index 879b920673..b78efcaf6a 100644 --- a/pandatool/src/win-stats/winStatsStripChart.cxx +++ b/pandatool/src/win-stats/winStatsStripChart.cxx @@ -103,7 +103,7 @@ new_data(int thread_index, int frame_number) { if (!_pause) { update(); - string text = format_number(get_average_net_value(), get_guide_bar_units(), get_guide_bar_unit_name()); + std::string text = get_total_text(); if (_net_value_text != text) { _net_value_text = text; RECT rect; From f54747c213c9dde405d9152d84b14d06541cefda Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 29 Nov 2022 14:23:32 +0100 Subject: [PATCH 06/14] makepanda: Make error about Android architecture suggest solution --- makepanda/makepandacore.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/makepanda/makepandacore.py b/makepanda/makepandacore.py index 405ce75fc0..56493f576e 100644 --- a/makepanda/makepandacore.py +++ b/makepanda/makepandacore.py @@ -434,7 +434,7 @@ def SetTarget(target, arch=None): ANDROID_ABI = 'x86_64' ANDROID_TRIPLE = 'x86_64-linux-android' else: - exit('Android architecture must be arm, armv7a, aarch64, mips, mips64, x86 or x86_64') + exit('Android architecture must be arm, armv7a, aarch64, mips, mips64, x86 or x86_64, use --arch to specify') TOOLCHAIN_PREFIX = ANDROID_TRIPLE + '-' From 7b9f2cd8548498580f3ff77a9290ad334771009e Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 29 Nov 2022 15:43:18 +0100 Subject: [PATCH 07/14] pstats: Support nested start/stop pairs in the server, accept 3.1 Currently, the client doesn't send nested start/stop pairs, but there is no good reason not to handle this case in the server, and in some cases it is useful to send them (I will soon make changes on master to take advantage of this). Client version 3.1 indicates that the client may send nested start/stop pairs. --- panda/src/pstatclient/pStatProperties.cxx | 7 +- pandatool/src/pstatserver/pStatReader.cxx | 3 +- pandatool/src/pstatserver/pStatView.cxx | 109 ++++++++++------------ 3 files changed, 55 insertions(+), 64 deletions(-) diff --git a/panda/src/pstatclient/pStatProperties.cxx b/panda/src/pstatclient/pStatProperties.cxx index cce31e693f..9537f50a90 100644 --- a/panda/src/pstatclient/pStatProperties.cxx +++ b/panda/src/pstatclient/pStatProperties.cxx @@ -27,9 +27,10 @@ using std::string; static const int current_pstat_major_version = 3; static const int current_pstat_minor_version = 0; -// Initialized at 2.0 on 51801, when version numbers were first added. -// Incremented to 2.1 on 52101 to add support for TCP frame data. Incremented -// to 3.0 on 42805 to bump TCP headers to 32 bits. +// Initialized at 2.0 on 5/18/01, when version numbers were first added. +// Incremented to 2.1 on 5/21/01 to add support for TCP frame data. +// Incremented to 3.0 on 4/28/05 to bump TCP headers to 32 bits. +// Incremented to 3.1 on 11/29/22 to support nested start/stop pairs. /** * Returns the current major version number of the PStats protocol. This is diff --git a/pandatool/src/pstatserver/pStatReader.cxx b/pandatool/src/pstatserver/pStatReader.cxx index 02b01f26a2..7c1fe6fca5 100644 --- a/pandatool/src/pstatserver/pStatReader.cxx +++ b/pandatool/src/pstatserver/pStatReader.cxx @@ -193,7 +193,8 @@ handle_client_control_message(const PStatClientControlMessage &message) { if (message._major_version != server_major_version || (message._major_version == server_major_version && - message._minor_version > server_minor_version)) { + message._minor_version > server_minor_version && + (message._major_version != 3 || message._minor_version != 1))) { _monitor->bad_version(message._client_hostname, message._client_progname, message._major_version, message._minor_version, server_major_version, server_minor_version); diff --git a/pandatool/src/pstatserver/pStatView.cxx b/pandatool/src/pstatserver/pStatView.cxx index 2e255273f7..092f74f276 100644 --- a/pandatool/src/pstatserver/pStatView.cxx +++ b/pandatool/src/pstatserver/pStatView.cxx @@ -26,61 +26,57 @@ */ class FrameSample { public: - void data_point(double time, bool is_start, FrameSample *started) { - _touched = true; + void start(double time, FrameSample *started) { + // Keep track of nested start/stop pairs. We only consider the outer one. + if (_started++ > 0) { + return; + } - // We only consider events that change the startstop state. With two - // consecutive 'start' events, for instance, we ignore the second one. + nassertv(!_pushed); + _net_time -= time; + push_all(time, started); + nassertv(_next == nullptr && _prev == nullptr); + _prev = started->_prev; + _next = started; + _prev->_next = this; + started->_prev = this; + } -/* - * *** That's not quite the right thing to do. We should keep track of the - * nesting level and bracket things correctly, so that we ignore the second - * start and the *first* stop, but respect the outer startstop. For the short - * term, this works, because the client is already doing this logic and won't - * send us nested startstop pairs, but we'd like to generalize this in the - * future so we can deal with these nested pairs properly. - */ - nassertv(is_start != _is_started); + void stop(double time, FrameSample *started) { + nassertv(_started > 0); + if (--_started > 0) { + return; + } - _is_started = is_start; + nassertv(_next != nullptr && _prev != nullptr); if (_pushed) { - nassertv(!_is_started); - nassertv(_next != nullptr && _prev != nullptr); _prev->_next = _next; _next->_prev = _prev; _next = _prev = nullptr; } else { - if (_is_started) { - _net_time -= time; - push_all(time, started); - nassertv(_next == nullptr && _prev == nullptr); - _prev = started->_prev; - _next = started; - _prev->_next = this; - started->_prev = this; - } else { - _net_time += time; - nassertv(_next != nullptr && _prev != nullptr); - _prev->_next = _next; - _next->_prev = _prev; - _next = _prev = nullptr; - pop_one(time, started); - } + _net_time += time; + _prev->_next = _next; + _next->_prev = _prev; + _next = _prev = nullptr; + pop_one(time, started); } } + +private: void push(double time) { if (!_pushed) { _pushed = true; - if (_is_started) { + if (_started > 0) { _net_time += time; } } } + void pop(double time) { if (_pushed) { _pushed = false; - if (_is_started) { + if (_started > 0) { _net_time -= time; } } @@ -103,14 +99,14 @@ public: } } +public: FrameSample *_next = nullptr; FrameSample *_prev = nullptr; - bool _touched = false; - bool _is_started = false; + double _net_time = 0.0; + int _started = 0; + int _count = 0; bool _pushed = false; bool _is_new = false; - int _count = 0; - double _net_time = 0.0; }; /** @@ -301,28 +297,21 @@ update_time_data(const PStatFrameData &frame_data) { if (_client_data->get_child_distance(_constraint, collector_index) >= 0) { // Here's a data point we care about: anything at constraint level or // below. - if (is_start == samples[collector_index]._is_started) { - if (!is_start) { - // A "stop" in the middle of a frame implies a "start" since time - // 0 (that is, since the first data point in the frame). - samples[collector_index].data_point(frame_data.get_time(0), true, &started); - samples[collector_index].data_point(frame_data.get_time(i), is_start, &started); - } else { - // An extra "start" for a collector that's already started is an - // error. - nout << "Unexpected data point for " - << _client_data->get_collector_fullname(collector_index) - << "\n"; - } + if (is_start) { + samples[collector_index].start(frame_data.get_time(i), &started); + samples[collector_index]._count++; } else { - samples[collector_index].data_point(frame_data.get_time(i), is_start, &started); - if (is_start) { - samples[collector_index]._count++; - } - if (!samples[collector_index]._is_new) { - samples[collector_index]._is_new = true; - ++new_collectors; + // A "stop" in the middle of a frame implies a "start" since time + // 0 (that is, since the first data point in the frame). + if (samples[collector_index]._started == 0) { + samples[collector_index].start(frame_data.get_time(0), &started); } + samples[collector_index].stop(frame_data.get_time(i), &started); + } + + if (!samples[collector_index]._is_new) { + samples[collector_index]._is_new = true; + ++new_collectors; } } } @@ -331,8 +320,8 @@ update_time_data(const PStatFrameData &frame_data) { // Make sure everything is stopped. Samples::iterator si; for (i = 0, si = samples.begin(); si != samples.end(); ++i, ++si) { - if ((*si)._is_started) { - (*si).data_point(frame_data.get_end(), false, &started); + if ((*si)._started > 0) { + (*si).stop(frame_data.get_end(), &started); } } From 50cfdebd9cfda3a349547a1898b05d81d977da3f Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 29 Nov 2022 16:00:09 +0100 Subject: [PATCH 08/14] pstats: Integrate Python profiler with PStats Enabled with `pstats-python-profiler 1`, requires recent version of PStats server. Currently, it is limited to the main thread only. Support for other threads may be added at a later date. --- dtool/src/interrogatedb/py_compat.h | 6 + dtool/src/parser-inc/Python.h | 3 + makepanda/makepanda.py | 2 + panda/src/pstatclient/config_pstatclient.cxx | 10 + panda/src/pstatclient/config_pstatclient.h | 1 + panda/src/pstatclient/pStatClient.h | 11 +- panda/src/pstatclient/pStatClientImpl.cxx | 7 + panda/src/pstatclient/pStatClient_ext.I | 31 ++ panda/src/pstatclient/pStatClient_ext.cxx | 339 +++++++++++++++++++ panda/src/pstatclient/pStatClient_ext.h | 49 +++ panda/src/pstatclient/pStatCollector.h | 4 +- panda/src/pstatclient/pStatThread.I | 8 + panda/src/pstatclient/pStatThread.h | 3 + 13 files changed, 467 insertions(+), 7 deletions(-) create mode 100644 panda/src/pstatclient/pStatClient_ext.I create mode 100644 panda/src/pstatclient/pStatClient_ext.cxx create mode 100644 panda/src/pstatclient/pStatClient_ext.h diff --git a/dtool/src/interrogatedb/py_compat.h b/dtool/src/interrogatedb/py_compat.h index 4ae393235d..35edfdec5a 100644 --- a/dtool/src/interrogatedb/py_compat.h +++ b/dtool/src/interrogatedb/py_compat.h @@ -213,6 +213,12 @@ INLINE PyObject *_PyLong_Lshift(PyObject *a, size_t shiftby) { } #endif +/* Python 3.9 */ + +#ifndef PyCFunction_CheckExact +# define PyCFunction_CheckExact(op) (Py_TYPE(op) == &PyCFunction_Type) +#endif + #if PY_VERSION_HEX < 0x03090000 INLINE EXPCL_PYPANDA PyObject *PyObject_CallNoArgs(PyObject *func) { return _PyObject_CallNoArg(func); diff --git a/dtool/src/parser-inc/Python.h b/dtool/src/parser-inc/Python.h index 5fa71246a5..ec9ad04008 100644 --- a/dtool/src/parser-inc/Python.h +++ b/dtool/src/parser-inc/Python.h @@ -25,6 +25,9 @@ typedef _object PyObject; struct _typeobject; typedef _typeobject PyTypeObject; +struct _frame; +typedef _frame PyFrameObject; + typedef struct {} PyStringObject; typedef struct {} PyUnicodeObject; diff --git a/makepanda/makepanda.py b/makepanda/makepanda.py index 49d0d74283..b2adc446e8 100755 --- a/makepanda/makepanda.py +++ b/makepanda/makepanda.py @@ -4228,6 +4228,7 @@ if (not RUNTIME): IGATEFILES.remove("config_pstats.h") TargetAdd('libp3pstatclient.in', opts=OPTS, input=IGATEFILES) TargetAdd('libp3pstatclient.in', opts=['IMOD:panda3d.core', 'ILIB:libp3pstatclient', 'SRCDIR:panda/src/pstatclient']) + PyTargetAdd('p3pstatclient_pStatClient_ext.obj', opts=OPTS, input='pStatClient_ext.cxx') # # DIRECTORY: panda/src/gobj/ @@ -4707,6 +4708,7 @@ if (not RUNTIME): PyTargetAdd('core.pyd', input='p3pnmimage_pfmFile_ext.obj') PyTargetAdd('core.pyd', input='p3event_asyncFuture_ext.obj') PyTargetAdd('core.pyd', input='p3event_pythonTask.obj') + PyTargetAdd('core.pyd', input='p3pstatclient_pStatClient_ext.obj') PyTargetAdd('core.pyd', input='p3gobj_ext_composite.obj') PyTargetAdd('core.pyd', input='p3pgraph_ext_composite.obj') PyTargetAdd('core.pyd', input='p3display_ext_composite.obj') diff --git a/panda/src/pstatclient/config_pstatclient.cxx b/panda/src/pstatclient/config_pstatclient.cxx index 1335254590..ecbf6baa79 100644 --- a/panda/src/pstatclient/config_pstatclient.cxx +++ b/panda/src/pstatclient/config_pstatclient.cxx @@ -82,6 +82,16 @@ ConfigVariableBool pstats_gpu_timing "is not usually an accurate reflectino of how long the actual " "operation takes on the video card.")); +ConfigVariableBool pstats_python_profiler +("pstats-python-profiler", false, + PRC_DESC("Set this true to integrate with the Python profiler to show " + "detailed information about individual Python functions in " + "PStats, similar to the information offered by Python's built-in " + "profiler. This can be really useful to find bottlenecks in a " + "Python program, but enabling this will slow down the application " + "somewhat, and requires a recent version of the PStats server, so " + "it is not enabled by default.")); + // The rest are different in that they directly control the server, not the // client. ConfigVariableBool pstats_scroll_mode diff --git a/panda/src/pstatclient/config_pstatclient.h b/panda/src/pstatclient/config_pstatclient.h index 6de3c5c120..936a253383 100644 --- a/panda/src/pstatclient/config_pstatclient.h +++ b/panda/src/pstatclient/config_pstatclient.h @@ -38,6 +38,7 @@ extern EXPCL_PANDA_PSTATCLIENT ConfigVariableString pstats_host; extern EXPCL_PANDA_PSTATCLIENT ConfigVariableInt pstats_port; extern EXPCL_PANDA_PSTATCLIENT ConfigVariableDouble pstats_target_frame_rate; extern EXPCL_PANDA_PSTATCLIENT ConfigVariableBool pstats_gpu_timing; +extern EXPCL_PANDA_PSTATCLIENT ConfigVariableBool pstats_python_profiler; extern EXPCL_PANDA_PSTATCLIENT ConfigVariableBool pstats_scroll_mode; extern EXPCL_PANDA_PSTATCLIENT ConfigVariableDouble pstats_history; diff --git a/panda/src/pstatclient/pStatClient.h b/panda/src/pstatclient/pStatClient.h index 1e6fe34480..7b356f6ad9 100644 --- a/panda/src/pstatclient/pStatClient.h +++ b/panda/src/pstatclient/pStatClient.h @@ -29,6 +29,7 @@ #include "atomicAdjust.h" #include "numeric_types.h" #include "bitArray.h" +#include "extension.h" class PStatClientImpl; class PStatCollector; @@ -88,8 +89,8 @@ PUBLISHED: MAKE_PROPERTY(current_thread, get_current_thread); MAKE_PROPERTY(real_time, get_real_time); - INLINE static bool connect(const std::string &hostname = std::string(), int port = -1); - INLINE static void disconnect(); + EXTEND INLINE static bool connect(const std::string &hostname = std::string(), int port = -1); + EXTEND INLINE static void disconnect(); INLINE static bool is_connected(); INLINE static void resume_after_pause(); @@ -99,8 +100,8 @@ PUBLISHED: void client_main_tick(); void client_thread_tick(const std::string &sync_name); - bool client_connect(std::string hostname, int port); - void client_disconnect(); + EXTEND bool client_connect(std::string hostname, int port); + EXTEND void client_disconnect(); bool client_is_connected() const; void client_resume_after_pause(); @@ -254,6 +255,8 @@ private: friend class PStatThread; friend class PStatClientImpl; friend class GraphicsStateGuardian; + + friend class Extension; }; #include "pStatClient.I" diff --git a/panda/src/pstatclient/pStatClientImpl.cxx b/panda/src/pstatclient/pStatClientImpl.cxx index ffdd5e2362..9f1f5a2ee6 100644 --- a/panda/src/pstatclient/pStatClientImpl.cxx +++ b/panda/src/pstatclient/pStatClientImpl.cxx @@ -399,6 +399,13 @@ send_hello() { message._major_version = get_current_pstat_major_version(); message._minor_version = get_current_pstat_minor_version(); + // The Python profiling feature may send nested start/stop pairs, so requires + // a server version capable of dealing with this. + if (pstats_python_profiler && message._major_version <= 3) { + message._major_version = 3; + message._minor_version = std::max(message._minor_version, 1); + } + Datagram datagram; message.encode(datagram); _writer.send(datagram, _tcp_connection, true); diff --git a/panda/src/pstatclient/pStatClient_ext.I b/panda/src/pstatclient/pStatClient_ext.I new file mode 100644 index 0000000000..c1c255d418 --- /dev/null +++ b/panda/src/pstatclient/pStatClient_ext.I @@ -0,0 +1,31 @@ +/** + * PANDA 3D SOFTWARE + * Copyright (c) Carnegie Mellon University. All rights reserved. + * + * All use of this software is subject to the terms of the revised BSD + * license. You should have received a copy of this license along + * with this source code in a file named "LICENSE." + * + * @file pStatClient_ext.I + * @author rdb + * @date 2022-11-29 + */ + +/** + * Attempts to establish a connection to the indicated PStatServer. Returns + * true if successful, false on failure. + */ +INLINE bool Extension:: +connect(const std::string &hostname, int port) { + PStatClient *client = PStatClient::get_global_pstats(); + return invoke_extension(client).client_connect(hostname, port); +} + +/** + * Closes the connection previously established. + */ +INLINE void Extension:: +disconnect() { + PStatClient *client = PStatClient::get_global_pstats(); + invoke_extension(client).client_disconnect(); +} diff --git a/panda/src/pstatclient/pStatClient_ext.cxx b/panda/src/pstatclient/pStatClient_ext.cxx new file mode 100644 index 0000000000..9d925660c3 --- /dev/null +++ b/panda/src/pstatclient/pStatClient_ext.cxx @@ -0,0 +1,339 @@ +/** + * PANDA 3D SOFTWARE + * Copyright (c) Carnegie Mellon University. All rights reserved. + * + * All use of this software is subject to the terms of the revised BSD + * license. You should have received a copy of this license along + * with this source code in a file named "LICENSE." + * + * @file pStatClient_ext.cxx + * @author rdb + * @date 2022-11-23 + */ + +#include "pStatClient_ext.h" +#include "pStatCollector.h" +#include "config_pstatclient.h" + +#include "frameobject.h" + +#if PY_VERSION_HEX >= 0x03060000 +static bool _python_profiler_enabled = false; + +// Used to cache stuff onto PyCodeObjects. +static Py_ssize_t _extra_index = -1; + +// Stores a mapping between C method definitions and collector indices. +static pmap _c_method_collectors; + +// Parent collector for all Python profiling collectors. +static PStatCollector code_collector("App:Python"); + +/** + * Walks up the type hierarchy to find the class where the method originates. + */ +static bool +find_method(PyTypeObject *&cls, PyObject *name, PyCodeObject *code) { + PyObject *meth = _PyType_Lookup(cls, name); + if (meth == nullptr || !PyFunction_Check(meth) || + PyFunction_GET_CODE(meth) != (PyObject *)code) { + return false; + } + + if (cls->tp_bases != nullptr) { + Py_ssize_t size = PyTuple_GET_SIZE(cls->tp_bases); + for (Py_ssize_t i = 0; i < size; ++i) { + PyTypeObject *base = (PyTypeObject *)PyTuple_GET_ITEM(cls->tp_bases, i); + + if (find_method(base, name, code)) { + cls = base; + return true; + } + } + } + + // Didn't find it in any of the bases, it must be defined here. + return true; +} + +/** + * Returns the collector for a Python frame. + */ +static int +#ifdef __GNUC__ +__attribute__ ((noinline)) +#elif defined(_MSC_VER) +__declspec(noinline) +#endif +make_python_frame_collector(PyFrameObject *frame, PyCodeObject *code) { +#if PY_VERSION_HEX >= 0x030B0000 // 3.11 + // Fetch the module name out of the frame's global scope. + PyObject *globals = PyFrame_GetGlobals(frame); + PyObject *py_mod_name = PyDict_GetItemString(globals, "__name__"); + Py_DECREF(globals); + + const char *mod_name = py_mod_name ? PyUnicode_AsUTF8(py_mod_name) : ""; + const char *meth_name = PyUnicode_AsUTF8(code->co_qualname); + char buffer[1024]; + size_t len = snprintf(buffer, sizeof(buffer), "%s:%s", mod_name, meth_name); + for (size_t i = 0; i < len - 1; ++i) { + if (buffer[i] == '.') { + buffer[i] = ':'; + } + } +#else + // Try to figure out the type name. There's no obvious way to do this. + // It's possible that the first argument passed to this function is the + // self instance or the current type (for a classmethod), but we have to + // double-check that to make sure. + PyTypeObject *cls = nullptr; + if (code->co_argcount >= 1) { + PyFrame_FastToLocals(frame); + PyObject *first_arg = PyDict_GetItem(frame->f_locals, PyTuple_GET_ITEM(code->co_varnames, 0)); + cls = PyType_Check(first_arg) ? (PyTypeObject *)first_arg : Py_TYPE(first_arg); + if ((cls->tp_flags & Py_TPFLAGS_HEAPTYPE) != 0) { + // Mangling scheme for methods starting (but not ending) with "__" + PyObject *meth_name = code->co_name; + Py_ssize_t len = PyUnicode_GET_LENGTH(meth_name); + if (len >= 2 && PyUnicode_READ_CHAR(meth_name, 0) == '_' && PyUnicode_READ_CHAR(meth_name, 1) == '_' && + (len < 4 || PyUnicode_READ_CHAR(meth_name, len - 1) != '_' || PyUnicode_READ_CHAR(meth_name, len - 2) != '_')) { + const char *cls_name = cls->tp_name; + while (cls_name[0] == '_') { + ++cls_name; + } + meth_name = PyUnicode_FromFormat("_%s%S", cls_name, meth_name); + } else { + Py_INCREF(meth_name); + } + if (!find_method(cls, meth_name, code)) { + // Not a matching method object, it's something else. Forget it. + cls = nullptr; + } + Py_DECREF(meth_name); + } else { + cls = nullptr; + } + } + + // Fetch the module name out of the frame's global scope. + PyObject *py_mod_name = PyDict_GetItemString(frame->f_globals, "__name__"); + if (py_mod_name == nullptr && cls != nullptr) { + py_mod_name = PyDict_GetItemString(cls->tp_dict, "__module__"); + } + + const char *mod_name = py_mod_name ? PyUnicode_AsUTF8(py_mod_name) : ""; + char buffer[1024]; + size_t len = snprintf(buffer, sizeof(buffer), "%s:", mod_name); + for (size_t i = 0; i < len - 1; ++i) { + if (buffer[i] == '.') { + buffer[i] = ':'; + } + } + + const char *meth_name = PyUnicode_AsUTF8(code->co_name); + if (cls != nullptr) { + len += snprintf(buffer + len, sizeof(buffer) - len, "%s:%s", cls->tp_name, meth_name); + } else { + len += snprintf(buffer + len, sizeof(buffer) - len, "%s", meth_name); + } +#endif + + // Add parentheses, unless it's something special like + if (len < sizeof(buffer) - 2 && buffer[len - 1] != '>') { + buffer[len++] = '('; + buffer[len++] = ')'; + buffer[len] = '\0'; + } + + PStatCollector collector(code_collector, buffer); + intptr_t collector_index = collector.get_index(); + if (_extra_index != -1) { + _PyCode_SetExtra((PyObject *)code, _extra_index, (void *)collector_index); + } + return collector_index; +} + +/** + * Creates a collector for a C function. + */ +static int +#ifdef __GNUC__ +__attribute__ ((noinline)) +#elif defined(_MSC_VER) +__declspec(noinline) +#endif +make_c_function_collector(PyCFunctionObject *meth) { + char buffer[1024]; + size_t len; + if (meth->m_self != nullptr && !PyModule_Check(meth->m_self)) { + PyTypeObject *cls = PyType_Check(meth->m_self) ? (PyTypeObject *)meth->m_self : Py_TYPE(meth->m_self); + + const char *dot = strrchr(cls->tp_name, '.'); + if (dot != nullptr) { + // The module name is included in the type name. + snprintf(buffer, sizeof(buffer), "%s:%s()", cls->tp_name, meth->m_ml->ml_name); + len = (dot - cls->tp_name) + 1; + } else { + // If there's no module name, we need to get it from __module__. + PyObject *py_mod_name = cls->tp_dict ? PyDict_GetItemString(cls->tp_dict, "__module__") : nullptr; + const char *mod_name; + if (py_mod_name != nullptr) { + mod_name = PyUnicode_AsUTF8(py_mod_name); + } else { + // Is it a built-in, like int or dict? + PyObject *builtins = PyEval_GetBuiltins(); + if (PyDict_GetItemString(builtins, cls->tp_name) == (PyObject *)cls) { + mod_name = "builtins"; + } else { + mod_name = ""; + } + } + len = snprintf(buffer, sizeof(buffer), "%s:%s:%s()", mod_name, cls->tp_name, meth->m_ml->ml_name) - 2; + } + } + else if (meth->m_self != nullptr) { + const char *mod_name = PyModule_GetName(meth->m_self); + len = snprintf(buffer, sizeof(buffer), "%s:%s()", mod_name, meth->m_ml->ml_name) - 2; + } + else { + snprintf(buffer, sizeof(buffer), "%s()", meth->m_ml->ml_name); + len = 0; + } + for (size_t i = 0; i < len; ++i) { + if (buffer[i] == '.') { + buffer[i] = ':'; + } + } + PStatCollector collector(code_collector, buffer); + int collector_index = collector.get_index(); + _c_method_collectors[meth->m_ml] = collector.get_index(); + return collector_index; +} +#endif // PY_VERSION_HEX + +/** + * Attempts to establish a connection to the indicated PStatServer. Returns + * true if successful, false on failure. + */ +bool Extension:: +client_connect(std::string hostname, int port) { +#if PY_VERSION_HEX >= 0x03060000 + extern struct Dtool_PyTypedObject Dtool_PStatThread; + + if (_this->client_connect(std::move(hostname), port)) { + // Pass a PStatThread as argument. + if (!_python_profiler_enabled && pstats_python_profiler) { + PStatThread *thread = new PStatThread(_this->get_current_thread()); + PyObject *arg = DTool_CreatePyInstance((void *)thread, Dtool_PStatThread, true, false); + if (_extra_index == -1) { + _extra_index = _PyEval_RequestCodeExtraIndex(nullptr); + } + PyEval_SetProfile(&trace_callback, arg); + _python_profiler_enabled = false; + } + return true; + } + else if (_python_profiler_enabled) { + PyEval_SetProfile(nullptr, nullptr); + _python_profiler_enabled = false; + } + return false; +#else + return _this->client_connect(std::move(hostname), port); +#endif +} + +/** + * Closes the connection previously established. + */ +void Extension:: +client_disconnect() { + _this->client_disconnect(); +#if PY_VERSION_HEX >= 0x03060000 + if (_python_profiler_enabled) { + PyEval_SetProfile(nullptr, nullptr); + _python_profiler_enabled = false; + } +#endif +} + +#if PY_VERSION_HEX >= 0x03060000 +/** + * Callback passed to PyEval_SetProfile. + */ +int Extension:: +trace_callback(PyObject *py_thread, PyFrameObject *frame, int what, PyObject *arg) { + intptr_t collector_index; + + if (what == PyTrace_CALL || what == PyTrace_RETURN || what == PyTrace_EXCEPTION) { + // Normal Python frame entry/exit. +#if PY_VERSION_HEX >= 0x030B0000 // 3.11 + PyCodeObject *code = PyFrame_GetCode(frame); +#else + PyCodeObject *code = frame->f_code; +#endif + + // The index for this collector is cached on the code object. + if (_PyCode_GetExtra((PyObject *)code, _extra_index, (void **)&collector_index) != 0 || collector_index == 0) { + collector_index = make_python_frame_collector(frame, code); + } + +#if PY_VERSION_HEX >= 0x030B0000 // 3.11 + Py_DECREF(code); +#endif + } else if (what == PyTrace_C_CALL || what == PyTrace_C_RETURN || what == PyTrace_C_EXCEPTION) { + // Call to a C function or method, which has no frame of its own. + if (PyCFunction_CheckExact(arg)) { + PyCFunctionObject *meth = (PyCFunctionObject *)arg; + auto it = _c_method_collectors.find(meth->m_ml); + if (it != _c_method_collectors.end()) { + collector_index = it->second; + } else { + collector_index = make_c_function_collector(meth); + } + } else { + return 0; + } + } else { + return 0; + } + + if (collector_index <= 0) { + return 0; + } + + PStatThread &pthread = *(PStatThread *)DtoolInstance_VOID_PTR(py_thread); + PStatClient *client = pthread.get_client(); + if (!client->client_is_connected()) { + // Client was disconnected, disable Python profiling. + PyEval_SetProfile(nullptr, nullptr); + _python_profiler_enabled = false; + return 0; + } + + int thread_index = pthread.get_index(); + +#ifdef _DEBUG + nassertr(collector_index >= 0 && collector_index < AtomicAdjust::get(_num_collectors), -1); + nassertr(thread_index >= 0 && thread_index < AtomicAdjust::get(_num_threads), -1); +#endif + + PStatClient::Collector *collector = client->get_collector_ptr(collector_index); + PStatClient::InternalThread *thread = client->get_thread_ptr(thread_index); + + if (collector->is_active() && thread->_is_active) { + double as_of = client->get_real_time(); + + LightMutexHolder holder(thread->_thread_lock); + if (thread->_thread_active) { + if (what == PyTrace_CALL || what == PyTrace_C_CALL) { + thread->_frame_data.add_start(collector_index, as_of); + } else { + thread->_frame_data.add_stop(collector_index, as_of); + } + } + } + + return 0; +} +#endif // PY_VERSION_HEX diff --git a/panda/src/pstatclient/pStatClient_ext.h b/panda/src/pstatclient/pStatClient_ext.h new file mode 100644 index 0000000000..3d703917dd --- /dev/null +++ b/panda/src/pstatclient/pStatClient_ext.h @@ -0,0 +1,49 @@ +/** + * PANDA 3D SOFTWARE + * Copyright (c) Carnegie Mellon University. All rights reserved. + * + * All use of this software is subject to the terms of the revised BSD + * license. You should have received a copy of this license along + * with this source code in a file named "LICENSE." + * + * @file pStatClient_ext.h + * @author rdb + * @date 2022-11-23 + */ + +#ifndef PSTATCLIENT_EXT_H +#define PSTATCLIENT_EXT_H + +#include "dtoolbase.h" + +#ifdef HAVE_PYTHON + +#include "extension.h" +#include "pStatClient.h" +#include "py_panda.h" + +typedef struct _frame PyFrameObject; + +/** + * This class defines the extension methods for PStatClient, which are called + * instead of any C++ methods with the same prototype. + */ +template<> +class Extension : public ExtensionBase { +public: + INLINE static bool connect(const std::string &hostname = std::string(), int port = -1); + INLINE static void disconnect(); + + bool client_connect(std::string hostname, int port); + void client_disconnect(); + +private: + static int trace_callback(PyObject *py_thread, PyFrameObject *frame, + int what, PyObject *arg); +}; + +#include "pStatClient_ext.I" + +#endif // HAVE_PYTHON + +#endif // PSTATCLIENT_EXT_H diff --git a/panda/src/pstatclient/pStatCollector.h b/panda/src/pstatclient/pStatCollector.h index aeb74f7cdb..2fa98c68cf 100644 --- a/panda/src/pstatclient/pStatCollector.h +++ b/panda/src/pstatclient/pStatCollector.h @@ -43,11 +43,9 @@ class Thread; class EXPCL_PANDA_PSTATCLIENT PStatCollector { #ifdef DO_PSTATS -private: - INLINE PStatCollector(PStatClient *client, int index); - public: PStatCollector() = default; + INLINE PStatCollector(PStatClient *client, int index); PUBLISHED: INLINE explicit PStatCollector(const std::string &name, diff --git a/panda/src/pstatclient/pStatThread.I b/panda/src/pstatclient/pStatThread.I index fb68d4d3f9..af1b8c8bff 100644 --- a/panda/src/pstatclient/pStatThread.I +++ b/panda/src/pstatclient/pStatThread.I @@ -82,3 +82,11 @@ INLINE int PStatThread:: get_index() const { return _index; } + +/** + * + */ +INLINE PStatClient *PStatThread:: +get_client() const { + return _client; +} diff --git a/panda/src/pstatclient/pStatThread.h b/panda/src/pstatclient/pStatThread.h index 74defa2a15..c07be30ad2 100644 --- a/panda/src/pstatclient/pStatThread.h +++ b/panda/src/pstatclient/pStatThread.h @@ -45,6 +45,9 @@ PUBLISHED: MAKE_PROPERTY(thread, get_thread); MAKE_PROPERTY(index, get_index); +public: + PStatClient *get_client() const; + private: PStatClient *_client; int _index; From adc324bbde2000059e2d9ca9f1071a1c49e7bf34 Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 29 Nov 2022 20:11:59 +0100 Subject: [PATCH 09/14] pstats: Fix assorted compile issues in more esoteric configurations --- panda/src/pstatclient/pStatClient_ext.cxx | 11 +++++++++-- panda/src/pstatclient/pStatClient_ext.h | 4 ++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/panda/src/pstatclient/pStatClient_ext.cxx b/panda/src/pstatclient/pStatClient_ext.cxx index 9d925660c3..db4b573c26 100644 --- a/panda/src/pstatclient/pStatClient_ext.cxx +++ b/panda/src/pstatclient/pStatClient_ext.cxx @@ -12,10 +12,15 @@ */ #include "pStatClient_ext.h" + +#if defined(HAVE_PYTHON) && defined(DO_PSTATS) + #include "pStatCollector.h" #include "config_pstatclient.h" +#ifndef CPPPARSER #include "frameobject.h" +#endif #if PY_VERSION_HEX >= 0x03060000 static bool _python_profiler_enabled = false; @@ -314,8 +319,8 @@ trace_callback(PyObject *py_thread, PyFrameObject *frame, int what, PyObject *ar int thread_index = pthread.get_index(); #ifdef _DEBUG - nassertr(collector_index >= 0 && collector_index < AtomicAdjust::get(_num_collectors), -1); - nassertr(thread_index >= 0 && thread_index < AtomicAdjust::get(_num_threads), -1); + nassertr(collector_index >= 0 && collector_index < AtomicAdjust::get(client->_num_collectors), -1); + nassertr(thread_index >= 0 && thread_index < AtomicAdjust::get(client->_num_threads), -1); #endif PStatClient::Collector *collector = client->get_collector_ptr(collector_index); @@ -337,3 +342,5 @@ trace_callback(PyObject *py_thread, PyFrameObject *frame, int what, PyObject *ar return 0; } #endif // PY_VERSION_HEX + +#endif // HAVE_PYTHON && DO_PSTATS diff --git a/panda/src/pstatclient/pStatClient_ext.h b/panda/src/pstatclient/pStatClient_ext.h index 3d703917dd..216d7777b1 100644 --- a/panda/src/pstatclient/pStatClient_ext.h +++ b/panda/src/pstatclient/pStatClient_ext.h @@ -16,7 +16,7 @@ #include "dtoolbase.h" -#ifdef HAVE_PYTHON +#if defined(HAVE_PYTHON) && defined(DO_PSTATS) #include "extension.h" #include "pStatClient.h" @@ -44,6 +44,6 @@ private: #include "pStatClient_ext.I" -#endif // HAVE_PYTHON +#endif // HAVE_PYTHON && DO_PSTATS #endif // PSTATCLIENT_EXT_H From e17b76f7755d0f1e3e213ed6ac50cefaeb9615d8 Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 29 Nov 2022 20:12:57 +0100 Subject: [PATCH 10/14] readme: Update links to point to 1.10.13 thirdparty packages [skip ci] --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 03c50bd2eb..8801356188 100644 --- a/README.md +++ b/README.md @@ -64,8 +64,8 @@ depending on whether you are on a 32-bit or 64-bit system, or you can [click here](https://github.com/rdb/panda3d-thirdparty) for instructions on building them from source. -- https://www.panda3d.org/download/panda3d-1.10.12/panda3d-1.10.12-tools-win64.zip -- https://www.panda3d.org/download/panda3d-1.10.12/panda3d-1.10.12-tools-win32.zip +- https://www.panda3d.org/download/panda3d-1.10.13/panda3d-1.10.13-tools-win64.zip +- https://www.panda3d.org/download/panda3d-1.10.13/panda3d-1.10.13-tools-win32.zip After acquiring these dependencies, you can build Panda3D from the command prompt using the following command. Change the `--msvc-version` option based From e5d36e2d018d3282a79ecfbb9de52ce673329335 Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 29 Nov 2022 20:13:49 +0100 Subject: [PATCH 11/14] makepackage: Add instructions for installing Python 3.12 bindings --- makepanda/installer.nsi | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/makepanda/installer.nsi b/makepanda/installer.nsi index 6696a0cf0b..fc0054b25a 100644 --- a/makepanda/installer.nsi +++ b/makepanda/installer.nsi @@ -425,6 +425,7 @@ SectionGroup "Python modules" SecGroupPython !insertmacro PyBindingSection 3.9-32 .cp39-win32.pyd !insertmacro PyBindingSection 3.10-32 .cp310-win32.pyd !insertmacro PyBindingSection 3.11-32 .cp311-win32.pyd + !insertmacro PyBindingSection 3.12-32 .cp312-win32.pyd !else !insertmacro PyBindingSection 3.5 .cp35-win_amd64.pyd !insertmacro PyBindingSection 3.6 .cp36-win_amd64.pyd @@ -433,6 +434,7 @@ SectionGroup "Python modules" SecGroupPython !insertmacro PyBindingSection 3.9 .cp39-win_amd64.pyd !insertmacro PyBindingSection 3.10 .cp310-win_amd64.pyd !insertmacro PyBindingSection 3.11 .cp311-win_amd64.pyd + !insertmacro PyBindingSection 3.12 .cp312-win_amd64.pyd !endif SectionGroupEnd @@ -543,6 +545,7 @@ Function .onInit !insertmacro MaybeEnablePyBindingSection 3.9-32 !insertmacro MaybeEnablePyBindingSection 3.10-32 !insertmacro MaybeEnablePyBindingSection 3.11-32 + !insertmacro MaybeEnablePyBindingSection 3.12-32 ${EndIf} !else !insertmacro MaybeEnablePyBindingSection 3.5 @@ -553,6 +556,7 @@ Function .onInit !insertmacro MaybeEnablePyBindingSection 3.9 !insertmacro MaybeEnablePyBindingSection 3.10 !insertmacro MaybeEnablePyBindingSection 3.11 + !insertmacro MaybeEnablePyBindingSection 3.12 ${EndIf} !endif @@ -570,6 +574,10 @@ Function .onInit SectionSetFlags ${SecPyBindings3.11} ${SF_RO} SectionSetInstTypes ${SecPyBindings3.11} 0 !endif + !ifdef SecPyBindings3.12 + SectionSetFlags ${SecPyBindings3.12} ${SF_RO} + SectionSetInstTypes ${SecPyBindings3.12} 0 + !endif ${EndUnless} FunctionEnd @@ -882,6 +890,7 @@ Section Uninstall !insertmacro RemovePythonPath 3.9-32 !insertmacro RemovePythonPath 3.10-32 !insertmacro RemovePythonPath 3.11-32 + !insertmacro RemovePythonPath 3.12-32 !else !insertmacro RemovePythonPath 3.5 !insertmacro RemovePythonPath 3.6 @@ -890,6 +899,7 @@ Section Uninstall !insertmacro RemovePythonPath 3.9 !insertmacro RemovePythonPath 3.10 !insertmacro RemovePythonPath 3.11 + !insertmacro RemovePythonPath 3.12 !endif SetDetailsPrint both @@ -966,6 +976,7 @@ SectionEnd !insertmacro MUI_DESCRIPTION_TEXT ${SecPyBindings3.9-32} $(DESC_SecPyBindings3.9-32) !insertmacro MUI_DESCRIPTION_TEXT ${SecPyBindings3.10-32} $(DESC_SecPyBindings3.10-32) !insertmacro MUI_DESCRIPTION_TEXT ${SecPyBindings3.11-32} $(DESC_SecPyBindings3.11-32) + !insertmacro MUI_DESCRIPTION_TEXT ${SecPyBindings3.12-32} $(DESC_SecPyBindings3.12-32) !else !insertmacro MUI_DESCRIPTION_TEXT ${SecPyBindings3.5} $(DESC_SecPyBindings3.5) !insertmacro MUI_DESCRIPTION_TEXT ${SecPyBindings3.6} $(DESC_SecPyBindings3.6) @@ -974,6 +985,7 @@ SectionEnd !insertmacro MUI_DESCRIPTION_TEXT ${SecPyBindings3.9} $(DESC_SecPyBindings3.9) !insertmacro MUI_DESCRIPTION_TEXT ${SecPyBindings3.10} $(DESC_SecPyBindings3.10) !insertmacro MUI_DESCRIPTION_TEXT ${SecPyBindings3.11} $(DESC_SecPyBindings3.11) + !insertmacro MUI_DESCRIPTION_TEXT ${SecPyBindings3.12} $(DESC_SecPyBindings3.12) !endif !ifdef INCLUDE_PYVER !insertmacro MUI_DESCRIPTION_TEXT ${SecPython} $(DESC_SecPython) From 44dd2bd09158ba917b85ca3fa5c6eecbb921d8e2 Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 29 Nov 2022 20:14:20 +0100 Subject: [PATCH 12/14] workflow: Update to 1.10.13 thirdparty packages --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0ab254bb7e..366d44ae38 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,9 +20,9 @@ jobs: shell: powershell run: | $wc = New-Object System.Net.WebClient - $wc.DownloadFile("https://www.panda3d.org/download/panda3d-1.10.11/panda3d-1.10.11-tools-win64.zip", "thirdparty-tools.zip") + $wc.DownloadFile("https://www.panda3d.org/download/panda3d-1.10.13/panda3d-1.10.13-tools-win64.zip", "thirdparty-tools.zip") Expand-Archive -Path thirdparty-tools.zip - Move-Item -Path thirdparty-tools/panda3d-1.10.11/thirdparty -Destination . + Move-Item -Path thirdparty-tools/panda3d-1.10.13/thirdparty -Destination . - name: Get thirdparty packages (macOS) if: runner.os == 'macOS' run: | From 5aad1d84045b2b0ab67035d5e7c90d3ffd3cbe39 Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 29 Nov 2022 20:14:33 +0100 Subject: [PATCH 13/14] interrogate: Change error message to mention interrogate This makes it more obvious that the error is coming from interrogate when encountering a parse error in a build log --- dtool/src/interrogate/interrogate.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dtool/src/interrogate/interrogate.cxx b/dtool/src/interrogate/interrogate.cxx index 2b2a756727..0d873762b1 100644 --- a/dtool/src/interrogate/interrogate.cxx +++ b/dtool/src/interrogate/interrogate.cxx @@ -513,7 +513,7 @@ main(int argc, char **argv) { for (i = 1; i < argc; ++i) { Filename filename = Filename::from_os_specific(argv[i]); if (!parser.parse_file(filename)) { - cerr << "Error parsing file: '" << argv[i] << "'\n"; + cerr << "interrogate failed to parse file: '" << argv[i] << "'\n"; exit(1); } builder.add_source_file(filename.to_os_generic()); From 0d3004567020a7c4dc6c669ae7d011b02973eb3b Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 29 Nov 2022 20:18:32 +0100 Subject: [PATCH 14/14] workflow: Update Python versions --- .github/workflows/ci.yml | 46 ++++++++++++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 366d44ae38..78f845c5e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,10 +31,39 @@ jobs: mv panda3d-1.10.13/thirdparty thirdparty rmdir panda3d-1.10.13 (cd thirdparty/darwin-libs-a && rm -rf rocket) - - name: Set up Python 3.9 - uses: actions/setup-python@v2 + + - name: Set up Python 3.11 + uses: actions/setup-python@v4 with: - python-version: 3.9 + python-version: '3.11' + - name: Build Python 3.11 + shell: bash + run: | + python makepanda/makepanda.py --git-commit=${{github.sha}} --outputdir=built --everything --no-eigen --python-incdir="$pythonLocation/include" --python-libdir="$pythonLocation/lib" --verbose --threads=4 --windows-sdk=10 + - name: Test Python 3.11 + shell: bash + run: | + python -m pip install pytest + PYTHONPATH=built LD_LIBRARY_PATH=built/lib DYLD_LIBRARY_PATH=built/lib python -m pytest + + - name: Set up Python 3.10 + uses: actions/setup-python@v4 + with: + python-version: '3.10' + - name: Build Python 3.10 + shell: bash + run: | + python makepanda/makepanda.py --git-commit=${{github.sha}} --outputdir=built --everything --no-eigen --python-incdir="$pythonLocation/include" --python-libdir="$pythonLocation/lib" --verbose --threads=4 --windows-sdk=10 + - name: Test Python 3.10 + shell: bash + run: | + python -m pip install pytest + PYTHONPATH=built LD_LIBRARY_PATH=built/lib DYLD_LIBRARY_PATH=built/lib python -m pytest + + - name: Set up Python 3.9 + uses: actions/setup-python@v4 + with: + python-version: '3.9' - name: Build Python 3.9 shell: bash run: | @@ -44,10 +73,11 @@ jobs: run: | python -m pip install pytest PYTHONPATH=built LD_LIBRARY_PATH=built/lib DYLD_LIBRARY_PATH=built/lib python -m pytest + - name: Set up Python 3.8 - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: - python-version: 3.8 + python-version: '3.8' - name: Build Python 3.8 shell: bash run: | @@ -57,10 +87,11 @@ jobs: run: | python -m pip install pytest PYTHONPATH=built LD_LIBRARY_PATH=built/lib DYLD_LIBRARY_PATH=built/lib python -m pytest + - name: Set up Python 3.7 - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: - python-version: 3.7 + python-version: '3.7' - name: Build Python 3.7 shell: bash run: | @@ -70,6 +101,7 @@ jobs: run: | python -m pip install pytest PYTHONPATH=built LD_LIBRARY_PATH=built/lib DYLD_LIBRARY_PATH=built/lib python -m pytest + - name: Make installer run: | python makepanda/makepackage.py --verbose --lzma