Implement debug memory marking #32949
Conversation
|
Will this not get overwritten by the OS / runtime on configurations that do memory marking themselves? If that is the case it wouldn't make sense in that situation, although I don't know if this environment can be detected from scons or not, or we can turn their memory marking off. If the OS / runtime is also marking, and we mark the memory with something different, anything we write that detects 'our' codes may not work if the codes are different, so there may be an argument for sharing the same codes, although yours may be more helpful for detecting certain errors in e.g. vectors. Here's some discussion of the microsoft ones: Personally I think such things can be very useful (also maybe something for detecting memory leaks), but I myself would like the option to be able to turn on and off irrespective of whether in debug or release build (might not want it slowing down debug builds unless I'm looking for such a bug, and might want it to catch a release build bug). Also there are of course numerous third party tools for doing this sort of thing (valgrind etc), so I would see the slant of such an approach towards areas that third party tools aren't as useful:
Or maybe the intention is simply to catch such bugs early on before PRs are merged, by enforcing it in debug? |
|
Yes, the idea is having a feature for catching memory bugs when you are working simply (compiler + debugger with no extra tools) and that works the same everywhere. At least, I've found this useful in the past (2.1). Now, it's certainly true that there are more sophisticated tools, but this shouldn't be a roadblock for using them. The Honestly, I'm not sure about which/how compilers/libraries will override this. I've tested it only on MSVC. If anyone has more insight about this, I'd really be happy to hear, so we can discuss how (and whether) to disable those and keep only this as the default one. |
-Added VulkanContext -Added an X11 implementation -Added a rendering device abstraction -added a Vulkan rendering device abstraction -Engine does not work, only shows Godot logo (run it from bin/)
* Implements a growing chunked allocator * Removed redudant methods get and getptr, only getornull is supported now.
-Texture renamed to Texture2D -TextureLayered as base now inherits 2Darray, cubemap and cubemap array -Removed all references to flags in textures (they will go in the shader) -Texture3D gone for now (will come back later done properly) -Create base rasterizer for RenderDevice, RasterizerRD
This should make it easier to obtain the data directly from an Image
…here. Removes antialiased flag for draw_* methods.
They should now allocate memory in blocks and reuse the same memory every time the item is cleared and redrawn. This should improve performance considerably.
Added support for Sprite, AnimatedSprite and Polygon2D (should add for tileset eventually).
Modified polygon management to make it more compatible with MoltenVK
…move GLSLang out.
Also, optimized shader compilation to happen on threads.
…more compatibility.
This consists in marking memory regions with certain specific values when they are (re)allocated or freed in order to ease debugging of certain bugs, catching them as early as possible, detect uninitialized variables, etc. Some C++ implementations may already do this in debug mode, but the idea of this is to make it work exactly the same regardless the platform/compiler/library. ``` ALLOCATED +-------------------------+ | F1 F1 F1 F1 F1 F1 F1 F1 | +-------------------------+ REALLOCATED (new size > old size) /----------------- new size -----------------/ /------- old size -------/---- size diff ----/ +------------------------+===================+ | current contents | F2 F2 F2 F2 F2 F2 | +------------------------+===================+ REALLOCATED (new size < old size) /----------------- old size -----------------/ /------- new size -------/---- size diff ----/ +------------------------+···················+ | kept contents | F3 F3 F3 F3 F3 F3 : +------------------------+···················+ FREED +·························+ : F4 F4 F4 F4 F4 F4 F4 F4 : +·························+ ```
RandomShaper
force-pushed
the
memory_marking
branch
from
64178c8
to
1322b66
Compare
|
@RandomShaper Is this still desired? If so, it needs to be rebased on the latest master branch (and the base branch needs to be changed to |
aaronfranke
requested review from
akien-mga,
AndreaCatania,
BastiaanOlij,
hpvb,
JFonS and
vnen
as code owners
aaronfranke
removed request for
BastiaanOlij,
hpvb,
JFonS,
vnen and
AndreaCatania
|
@RandomShaper Is this still desired? If so, it needs to be rebased on the latest master branch. If not, abandoned pull requests will be closed in the future as announced here. |
|
I have in my backlog plans to revive it, since I think it's really useful to catch and understand some memory errors, especially on platforms were you can't easily build with asan. |
|
I'm letting this go due to the lack of interest. |
|
If anyone is interested in salvaging this PR, this proposal is also related: godotengine/godot-proposals#674 |
This consists in marking memory regions with certain specific values when they are (re)allocated or
freed in order to ease debugging of certain bugs, catching them as early as possible, detect
uninitialized variables, etc.
Some C++ implementations may already do this in debug mode, but the idea of this is to make it work
exactly the same regardless the platform/compiler/library.