Opened 12 years ago
Closed 11 years ago
#2006 closed enhancement (wontfix)
[PATCH] CModelDef slow/memory intensive load
| Reported by: | Jorma Rebane | Owned by: | Jorma Rebane |
|---|---|---|---|
| Priority: | Must Have | Milestone: | |
| Component: | Core engine | Keywords: | patch performance memory |
| Cc: | Patch: |
Description
Check original Ticket: #1995
CModelDef fixes uploaded. This patch addresses two problems: 1) CModelDef loading performance
In order to make loading faster, had to change the format of cached binary model data. The new model cache files take a tiny bit more disk space, but in return they load several times faster and the loaded data takes less memory.
2) Complex dynamic objects in structs
One of the main perf hitters was a per-vertex (yes, per each vertex!) std::vector<float> for multiple UV's. This was replaced with 2x CVector2D UV0 & UV1. Another one was an std::string for model prop points, which was replaced with a fixed size buffer m_Name[20] and with an additional m_NameLength.
The result: much better memory performance and usage for model loading.
In order for this to work, I had to change the version of the PMD (binary model cache) format to 4 and change the way models are saved to PMD. This may invalidate any existing model cache. Backwards compatibility for versions 1, 2, 3 is there.
Attachments (1)
Change History (9)
comment:1 by , 12 years ago
| Summary: | CModelDef slow/memory intensive load → [PATCH] CModelDef slow/memory intensive load |
|---|
comment:2 by , 12 years ago
| Keywords: | patch review added; model performance memory load removed |
|---|
comment:3 by , 12 years ago
| Keywords: | patch, review → patch review |
|---|
comment:4 by , 12 years ago
| Milestone: | Backlog → Alpha 14 |
|---|
comment:5 by , 12 years ago
| Keywords: | performance memory added |
|---|
by , 11 years ago
| Attachment: | CModelDef.patch added |
|---|
comment:6 by , 11 years ago
Some comments, though generally this sounds good to me, and looking at the patch I see nothing outlandish:
- 20 characters for the prop point name might be a bit too small. "horse_rider_projectile" is already above. I'd say 30/40 would be safe enough. Such a hard limitations also needs to be very explicit. I'm assuming it would throw an error with a longer name, and I think there should be an easy to understand error message
- your patch as a "no newline at end of file" bit at the end of "ps/filelo.cpp"
(also, this is fairly random, but most patches use the "trunk" directory as the root, yours uses "Source").However, I'me getting "EXC_BAD_ACCESS" crashes on many maps with it applied, apparently caused by "unpacker.UnpackRaw(vertices, sizeof(SModelVertex) * numVertices);" in ModelDef.cpp. Belgian Bog loads, but not other maps such as Acropolis.
comment:7 by , 11 years ago
| Milestone: | Alpha 14 → Alpha 15 |
|---|
comment:8 by , 11 years ago
| Keywords: | review removed |
|---|---|
| Milestone: | Alpha 15 |
| Resolution: | → wontfix |
| Status: | new → closed |
Closing these tickets as no further active development is expected. See Philip's megapatch-split git branch for an attempt at splitting megapatch into it's separate parts: http://git.wildfiregames.com/gitweb/?p=0ad.git;a=shortlog;h=refs/heads/projects/philip/megapatch-split

Updated patch. Removed size_t 32/64-bit bug.