GNOME Bugzilla – Bug 621505
Disable memory poisoning by default for releases
Last modified: 2010-06-23 09:00:17 UTC
Currently memory poisoning is always enabled. While this helps tracking down issue in development versions, it does enable some non-trivial computations which slow down caps operations.
gst_caps_append: Full caps_copy of the second argument
gst_caps_merge: Full caps copy of the second argumet
gst_caps_append_structure: Full copy of the structure to add
I'd push for not making it enabled by default for (pre-)releases.
Agreed, let's disable it for (pre-) releases.
even worse than what I expected, it's also called by every API method calling those methods above (like gst_caps_intersect for ex :()
How many bugs do we catch due to poisoning anyway? Don't we usually catch those bugs with valgrind?
> Currently memory poisoning is always enabled.
I'm not sure this is true in general. It's enabled in git because --enable-poisoning is one of the default args passed to configure in autogen.sh. It's not enabled if you unpack a tarball and run ./configure ...
gah, good point. It's autogen.sh the culprit :(
As Benjamin noted though, is there still a point to have memory poisoning ?
> As Benjamin noted though, is there still a point to have memory poisoning ?
Benjamin lives in a happy place where he's got full control of the stack and OS and valgrind works well and is generally fast enough. Other people may not be quite so lucky and may occasionally have to wrestle with puny embedded systems, proprietary operating systems etc.
I think it'd be nice to keep the option around to make debugging easier, even if we decide not to use it any more by default.
Ok, so any objections for removing it from autogen.sh then but still keeping the option around for those who want/need it ?
The overwriting of GstCaps/GstStructure structures caught several bugs when it was added. It probably continues to have minor value by triggering valgrind warnings or segfaults earlier because of errors in external code, and the cost should be minor since the structure sizes are small and are likely in cache. I'd like this to continue to be on, either via autogen.sh or GST_CVS=yes. (Better would be a g_free variant that does poisoning based on an environment variable, but I digress.)
Copying the caps/structures at every operation seems rather expensive in comparison. I'm probably missing something, but this seems to be geared more toward checking for bugs in core, not bugs in external code.
I also think we should keep the current behaviour.
It's already disabled by default for releases, which is the most important thing I guess, so can we close this?
Yah, let's close it. As long as people are aware of it when profiling on non-release checkouts I'm fine with it.