Skip to content
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.

Sign up for GitHub

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

Jump to bottom

remove query stuff from record page #454

Merged
merged 7 commits into from
Sep 1, 2023
Merged

remove query stuff from record page #454

merged 7 commits into from
Sep 1, 2023

Conversation

lievenhey
Copy link
Contributor

@lievenhey lievenhey commented Jan 4, 2023 •
edited
Loading

rewrite record page to query data asynchronously from RecordHost. This will make implementing ssh much more easier.

@lievenhey lievenhey marked this pull request as draft January 4, 2023 14:18
@lievenhey lievenhey force-pushed the record-host branch 6 times, most recently from 427a893 to d159b17 Compare January 5, 2023 13:50
@lievenhey lievenhey changed the title first version of record host remove query stuff from record page Jan 5, 2023
@lievenhey lievenhey marked this pull request as ready for review January 5, 2023 13:51
@lievenhey lievenhey requested a review from milianw January 5, 2023 13:51
@GitMensch
Copy link
Contributor

Note: the current AppImage for this ./hotspot-v1.4.0-42-gd159b17-x86_64.AppImage sigsegvs on start (while ./hotspot-v1.4.1-44-g46244df-x86_64.AppImage from the disassembly PR starts fine).

Can you please rebase this? Is this related to the KF version increase?

@lievenhey
Copy link
Contributor Author

It shouldn't since we build with kf5 version 98+

@GitMensch
Copy link
Contributor

Does the AppImage chrash on your side, too?

@lievenhey
Copy link
Contributor Author

yes, but if I build it from source it works. Still need some time to investigate.

@GitMensch
Copy link
Contributor

Only "minor investigation": stacktrace from extracted appimage with the debug symbols manually loaded:

#/tmp/squashfs-root/usr/bin> gdb -q ./hotspot
Reading symbols from ./hotspot...

warning: Loadable section ".dynstr" outside of ELF segments
  in /tmp/squashfs-root/usr/bin/hotspot
(No debugging symbols found in ./hotspot)
(gdb) symbol-file /tmp/hotspot-debuginfo-v1.4.0-43-g02fd82c/usr/bin/hotspot
Reading symbols from /tmp/hotspot-debuginfo-v1.4.0-43-g02fd82c/usr/bin/hotspot...
(gdb) run
Starting program: /tmp/squashfs-root/usr/bin/hotspot
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib64/libthread_db.so.1".
[New Thread 0x7fffdbbc9700 (LWP 3736130)]
QStandardPaths: XDG_RUNTIME_DIR not set, defaulting to '/tmp/runtime-so'
QStandardPaths: XDG_RUNTIME_DIR not set, defaulting to '/tmp/runtime-so'
[New Thread 0x7fffd3fff700 (LWP 3736324)]
[New Thread 0x7fffcb7fe700 (LWP 3736325)]
[New Thread 0x7fffd37fe700 (LWP 3736326)]
[New Thread 0x7fffd2ffd700 (LWP 3736327)]
[New Thread 0x7fffd27fc700 (LWP 3736328)]
[New Thread 0x7fffd1ffb700 (LWP 3736329)]
[New Thread 0x7fffd17fa700 (LWP 3736330)]
[New Thread 0x7fffd0ff9700 (LWP 3736331)]
[New Thread 0x7fffcbfff700 (LWP 3736332)]
[Detaching after fork from child process 3736333]
ASSERT failure in KConfigGroup::readEntry: "accessing an invalid group", file /opt/kde/src/frameworks/kconfig/src/core/kconfiggroup.cpp, line 675

Thread 1 "hotspot" received signal SIGABRT, Aborted.
0x00007ffff3efb37f in raise () from /usr/lib64/libc.so.6
(gdb) bt
#0  0x00007ffff3efb37f in raise () from /usr/lib64/libc.so.6
#1  0x00007ffff3ee5db5 in abort () from /usr/lib64/libc.so.6
#2  0x00007ffff4c53c81 in QMessageLogger::fatal(char const*, ...) const ()
   from /tmp/squashfs-root/usr/bin/../lib/libQt5Core.so.5
#3  0x00007ffff4c5321e in qt_assert_x(char const*, char const*, char const*, int) ()
   from /tmp/squashfs-root/usr/bin/../lib/libQt5Core.so.5
#4  0x00007ffff63f1095 in KConfigGroup::readEntry(char const*, QString const&) const ()
   from /tmp/squashfs-root/usr/bin/../lib/libKF5ConfigCore.so.5
#5  0x000000000048855a in operator() (filePath=..., __closure=0x728c70) at /__w/hotspot/hotspot/src/recordpage.cpp:171
#6  QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<const QString&>, void, RecordPage::RecordPage(QWidget*)::<lambda(const QString&)> >::call (arg=<optimized out>, f=...) at /usr/include/QtCore/qobjectdefs_impl.h:146
#7  QtPrivate::Functor<RecordPage::RecordPage(QWidget*)::<lambda(const QString&)>, 1>::call<QtPrivate::List<QString const&>, void>
    (arg=<optimized out>, f=...) at /usr/include/QtCore/qobjectdefs_impl.h:256
#8  QtPrivate::QFunctorSlotObject<RecordPage::RecordPage(QWidget*)::<lambda(const QString&)>, 1, QtPrivate::List<const QString&>, void>::impl(int, QtPrivate::QSlotObjectBase *, QObject *, void **, bool *) (which=<optimized out>, this_=0x728c60,
    r=<optimized out>, a=<optimized out>, ret=<optimized out>) at /usr/include/QtCore/qobjectdefs_impl.h:443
#9  0x00007ffff4ea316d in ?? () from /tmp/squashfs-root/usr/bin/../lib/libQt5Core.so.5
#10 0x000000000043c355 in RecordHost::clientApplicationChanged (this=this@entry=0x6cc020, _t1=...)
    at /__w/hotspot/hotspot/build/src/hotspot_autogen/EWIEGA46WW/moc_recordhost.cpp:294
#11 0x00000000004c183f in RecordHost::setHost (this=0x6cc020, host=...) at /__w/hotspot/hotspot/src/recordhost.cpp:205
#12 0x000000000048a67a in RecordPage::RecordPage (this=this@entry=0x6cbf70, parent=parent@entry=0x647af0)
    at /__w/hotspot/hotspot/src/recordpage.cpp:215
#13 0x0000000000471467 in MainWindow::MainWindow (this=this@entry=0x647af0, parent=parent@entry=0x0, __in_chrg=<optimized out>,
    __vtt_parm=<optimized out>) at /__w/hotspot/hotspot/src/mainwindow.cpp:104
#14 0x000000000043b187 in main (argc=<optimized out>, argv=<optimized out>) at /__w/hotspot/hotspot/src/main.cpp:195
(gdb)

milianw
milianw requested changes Jan 12, 2023
View reviewed changes
src/jobtracker.h Outdated Show resolved Hide resolved
src/recordhost.cpp Outdated Show resolved Hide resolved
src/recordhost.cpp Outdated Show resolved Hide resolved
src/recordhost.cpp Outdated Show resolved Hide resolved
src/recordhost.cpp Outdated Show resolved Hide resolved
src/util.cpp Outdated Show resolved Hide resolved
tests/integrationtests/tst_perfparser.cpp Outdated Show resolved Hide resolved
tests/integrationtests/tst_perfparser.cpp Outdated Show resolved Hide resolved
tests/integrationtests/tst_perfparser.cpp Outdated Show resolved Hide resolved
tests/integrationtests/tst_perfparser.cpp Outdated Show resolved Hide resolved
@lievenhey lievenhey force-pushed the record-host branch 3 times, most recently from d6c5c76 to 9a07689 Compare March 1, 2023 14:46
@GitMensch
Copy link
Contributor

@lievenhey Can you please give a an overview where this PR stands? I guess #359 waits for this to be handled first.

@lievenhey
Copy link
Contributor Author

I am currently waiting for the new elevate system. Once that is merged, I will tackle this and then ssh

@GitMensch
Copy link
Contributor

Thank you, so #493 comes before this and this becomes before ssh - thank you for the clarification.

@lievenhey lievenhey force-pushed the record-host branch 2 times, most recently from bf91f37 to bf1811e Compare August 2, 2023 11:22
@GitMensch
Copy link
Contributor

@milianw: seems like all of your review comments are handled, so possibly ready for pull?

@GitMensch
Copy link
Contributor

.. but docs (screenshots) should likely be adjusted either here or with a follow-up PR.

@lievenhey lievenhey force-pushed the record-host branch 3 times, most recently from 08e30c5 to cc2c9fd Compare August 29, 2023 15:32
milianw
milianw requested changes Aug 29, 2023
View reviewed changes
src/recordhost.cpp Show resolved Hide resolved
src/util.cpp Outdated Show resolved Hide resolved
src/recordhost.cpp Outdated Show resolved Hide resolved
@lievenhey @milianw
bump required version of kf5 to 5.42.0
f4a310c 
nowadays we build everything ourself so we don't need to keep the
workaround for 5 year old patches around
@milianw milianw force-pushed the record-host branch 5 times, most recently from 793819f to 8cadf77 Compare August 30, 2023 14:03
@milianw
Always compile tst_callgraphgenerator
0fa1a06 
It doesn't depend on the viewer to be available
@lievenhey lievenhey force-pushed the record-host branch 2 times, most recently from 9dce5fb to 7f6d4f6 Compare September 1, 2023 12:57
@lievenhey
add perf query interface and untangle it from recordpage
fd3c506 
moving the query part of the recordpage into its own class will make
implementing remote recording much easier
@lievenhey
fix tst_callgraphgenerator calling perf
f847afe 
this doesn't work inside a docker container so I added a perfparser dump
of the data to process
@lievenhey
remove unused include
1f7c89e
@milianw
Fix race condition when setting thread names in perfparser
2dbbf1d 
We must not write to any data member from the background thread.
Instead, use a signal to serialize the write operation into the
context of the main thread. This fixes the data race and potential
issues when running this code.
@milianw
Emit parsing finished at the very end of the background thread
0121830 
Otherwise, we might get unlucky and crash when our tests await this
signal and then destroy the parser object as a result, leading to
crashes.
@milianw milianw merged commit 747fed4 into master Sep 1, 2023
23 checks passed
@milianw milianw deleted the record-host branch September 1, 2023 14:08
@milianw
Copy link
Member

milianw commented Sep 1, 2023

.. but docs (screenshots) should likely be adjusted either here or with a follow-up PR.

nothing user-visible is changed in this change, it's purely internal

... except that we now can configure the path to the perf that should be used - but that's so minor, we don't need to have this imo :) that said, patches welcome obviously if you want to document this!

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

@milianw milianw milianw requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@lievenhey @GitMensch @milianw