Fix: AbstractMetaBuilder::classesTopologicalSorted #148
Conversation
mrbean-bremen
force-pushed
the
make_generator_work_for_5.15
branch
from
7be0dd9 to
d476532
Compare
This explicit new/delete, storage leaks (missing delete) and iteration over a QHash while modifying it (deleting specific keys in an iterator). Signed-off-by: John Bowler <[email protected]>
There was a problem hiding this comment.
This looks much cleaner than before, especially as it avoids the new / delete stuff. I'm not sure about the performance - the numbers seen in the CI are not really conclusive - but I don't really care as much about performance here as about correctness and stability.
That's why I started looking at it; the GCC with -pg showed this one function (including the things it calls) as accounting for a very large percentage of the run time (I can't remember the number but it was around half). After I'd done the changes the % dropped by a factor of 5. However gprof lies; the actual run-time of the generator was not changed significantly! I still don't understand where the perf is going but "-p" profiling seems to have been dropped from the GCC (or maybe I don't know where it is) and I don't have anything on my Gentoo system to do sampled profiling (where the PC is sampled regularly then mapped back to the function based on the debug information). I always found that much better and VS supported it really well (this was 20 years ago). You could try profiling a run under VS but I don't know which version you are using; good support probably requires Pro and the best support needed the Enterprise version :-( |
|
As I wrote, I don't really care much about the performance, as the generator will usually be run only once for a version to generate the code (except in the CI, of course), which is then used. Or if the checked in generated code is used, it need not to be run at all. Having a version that creates correct code and is stable is much more important from my point of view. As I wrote elsewhere, if we can get it to a stage where 5.15 sources are generated reliably, we can newly check in the newly generated code and call it a release. What bugs me a bit are the crashes you get on your system with Qt6, but that may not be relevant for the Qt 5.15 generator. |
|
Github has a weird interface. I think I double clicked something. |
Signed-off-by: John Bowler <[email protected]>
If it crashes, even if it crashes with invalid input, it's a problem. |
|
I must have mis-tested this before; this change does make the two crashes I was seeing go away. I can't be sure if it fixes them of course; they may be intermittent, but with the current make_generator* branch I get crashes but with this change I didn't see them. Since I can repro the crashes I can probably find out where they are but that may not help if it's heap corruption. Anyway I can apparently repro with commit 94c8ee2 reliably so it's safe to merge this if the CI checks pass now. EDIT: I had accidentally rebased the PR against master, so I was testing master with my change. |
mrbean-bremen
merged commit bd4fe2c
into
MeVisLab:make_generator_work_for_5.15
This explicit new/delete, storage leaks (missing delete) and iteration over a QHash while modifying it (deleting specific keys in an iterator).
This explicit new/delete, storage leaks (missing delete) and iteration over a QHash while modifying it (deleting specific keys in an iterator).
This fixes explicit new/delete, storage leaks (missing delete) and iteration over a QHash while modifying it (deleting specific keys in an iterator).
Tested against the current make_generator_work_for_5.15 branch; it generates identical output and crashes in the same places (6.5.3).