Skip to content

Commit 7edb758

Browse files
committed
Address blob lifecycle review feedback
Remove the extra LoadFile byte copy by moving the runtime file-loader buffer directly into the blob. Replace unbounded tombstone storage with reusable slot/generation handles and add churn coverage for stale-handle validation. Normalize blob source extensions before format inference and cover mixed-case metadata.
1 parent 983543a commit 7edb758

7 files changed

Lines changed: 176 additions & 84 deletions

File tree

source/common/AssetSuite.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ AssetSuite::Result AssetSuite::LoadFile(ContextHandle context, const char* fileP
152152
return Result::ErrorInvalidArgument;
153153
}
154154

155-
std::vector<BYTE> rawBytes;
155+
std::vector<uint8_t> rawBytes;
156156
const ErrorCode loadResult = context->Runtime().FileLoader().LoadToMemory(filePath, true, rawBytes);
157157
const Result mappedResult = MapFileLoadResult(loadResult);
158158
if (mappedResult != Result::Success)
@@ -163,8 +163,7 @@ AssetSuite::Result AssetSuite::LoadFile(ContextHandle context, const char* fileP
163163

164164
try
165165
{
166-
std::vector<uint8_t> bytes(rawBytes.begin(), rawBytes.end());
167-
Internal::Blob blob(std::move(bytes), Internal::MakeBlobSourceMetadata(filePath));
166+
Internal::Blob blob(std::move(rawBytes), Internal::MakeBlobSourceMetadata(filePath));
168167
*outBlob = context->Runtime().BlobStorage().Create(std::move(blob));
169168
}
170169
catch (const std::bad_alloc&)

source/runtime/AssetSuiteBlob.cpp

Lines changed: 17 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,40 @@
11
#include "AssetSuiteBlob.h"
22

3+
#include <algorithm>
4+
#include <cctype>
5+
#include <string>
36
#include <utility>
47

58
namespace
69
{
710
AssetSuite::AssetFormat ResolveBlobFormat(const std::filesystem::path& extension) noexcept
811
{
9-
if (extension == ".bmp")
12+
std::string normalizedExtension = extension.string();
13+
std::transform(
14+
normalizedExtension.begin(),
15+
normalizedExtension.end(),
16+
normalizedExtension.begin(),
17+
[](unsigned char character)
18+
{
19+
return static_cast<char>(std::tolower(character));
20+
});
21+
22+
if (normalizedExtension == ".bmp")
1023
{
1124
return AssetSuite::AssetFormat::BMP;
1225
}
1326

14-
if (extension == ".png")
27+
if (normalizedExtension == ".png")
1528
{
1629
return AssetSuite::AssetFormat::PNG;
1730
}
1831

19-
if (extension == ".ppm")
32+
if (normalizedExtension == ".ppm")
2033
{
2134
return AssetSuite::AssetFormat::PPM;
2235
}
2336

24-
if (extension == ".obj")
37+
if (normalizedExtension == ".obj")
2538
{
2639
return AssetSuite::AssetFormat::WavefrontObj;
2740
}
@@ -76,30 +89,3 @@ AssetSuite::Internal::MakeBlobSourceMetadata(const std::filesystem::path& source
7689
metadata.format = ResolveBlobFormat(metadata.extension);
7790
return metadata;
7891
}
79-
80-
AssetSuite::AssetSuiteBlob_t::AssetSuiteBlob_t(Internal::Blob blob)
81-
: blob(std::make_unique<Internal::Blob>(std::move(blob)))
82-
{
83-
}
84-
85-
AssetSuite::AssetSuiteBlob_t::~AssetSuiteBlob_t() = default;
86-
87-
AssetSuite::Internal::Blob& AssetSuite::AssetSuiteBlob_t::Blob() noexcept
88-
{
89-
return *blob;
90-
}
91-
92-
const AssetSuite::Internal::Blob& AssetSuite::AssetSuiteBlob_t::Blob() const noexcept
93-
{
94-
return *blob;
95-
}
96-
97-
bool AssetSuite::AssetSuiteBlob_t::IsLive() const noexcept
98-
{
99-
return blob != nullptr;
100-
}
101-
102-
void AssetSuite::AssetSuiteBlob_t::Release() noexcept
103-
{
104-
blob.reset();
105-
}

source/runtime/AssetSuiteBlob.h

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
#include <cstdint>
88
#include <filesystem>
9-
#include <memory>
109
#include <vector>
1110

1211
namespace AssetSuite::Internal
@@ -45,18 +44,5 @@ namespace AssetSuite
4544
{
4645
struct AssetSuiteBlob_t
4746
{
48-
explicit AssetSuiteBlob_t(Internal::Blob blob);
49-
~AssetSuiteBlob_t();
50-
51-
AssetSuiteBlob_t(const AssetSuiteBlob_t&) = delete;
52-
AssetSuiteBlob_t& operator=(const AssetSuiteBlob_t&) = delete;
53-
54-
Internal::Blob& Blob() noexcept;
55-
const Internal::Blob& Blob() const noexcept;
56-
bool IsLive() const noexcept;
57-
void Release() noexcept;
58-
59-
private:
60-
std::unique_ptr<Internal::Blob> blob;
6147
};
6248
}

source/runtime/AssetSuiteRuntimeState.cpp

Lines changed: 105 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,48 @@
88

99
#include <fstream>
1010
#include <new>
11+
#include <cstdint>
1112
#include <utility>
1213

14+
namespace
15+
{
16+
constexpr uintptr_t SLOT_INDEX_MASK = 0xffffffffu;
17+
constexpr uint32_t INITIAL_BLOB_GENERATION = 1;
18+
19+
AssetSuite::BlobHandle EncodeBlobHandle(size_t slotIndex, uint32_t generation) noexcept
20+
{
21+
const uintptr_t token =
22+
(static_cast<uintptr_t>(generation) << 32) |
23+
(static_cast<uintptr_t>(slotIndex) + 1u);
24+
return reinterpret_cast<AssetSuite::BlobHandle>(token);
25+
}
26+
27+
bool DecodeBlobHandle(AssetSuite::BlobHandle handle, size_t& slotIndex, uint32_t& generation) noexcept
28+
{
29+
const uintptr_t token = reinterpret_cast<uintptr_t>(handle);
30+
const uintptr_t encodedSlotIndex = token & SLOT_INDEX_MASK;
31+
if (encodedSlotIndex == 0)
32+
{
33+
return false;
34+
}
35+
36+
generation = static_cast<uint32_t>(token >> 32);
37+
if (generation == 0)
38+
{
39+
return false;
40+
}
41+
42+
slotIndex = static_cast<size_t>(encodedSlotIndex - 1u);
43+
return true;
44+
}
45+
46+
uint32_t NextBlobGeneration(uint32_t generation) noexcept
47+
{
48+
++generation;
49+
return generation == 0 ? INITIAL_BLOB_GENERATION : generation;
50+
}
51+
}
52+
1353
AssetSuite::Internal::RuntimeState::RuntimeState()
1454
: codecs()
1555
{
@@ -81,7 +121,7 @@ AssetSuite::Internal::RuntimeState::Diagnostics::Entries() const noexcept
81121
AssetSuite::ErrorCode AssetSuite::Internal::RuntimeState::FileLoader::LoadToMemory(
82122
const std::filesystem::path& fileName,
83123
bool isBinary,
84-
std::vector<BYTE>& output) const
124+
std::vector<uint8_t>& output) const
85125
{
86126
if (!std::filesystem::exists(fileName))
87127
{
@@ -118,48 +158,89 @@ AssetSuite::ErrorCode AssetSuite::Internal::RuntimeState::FileLoader::LoadToMemo
118158

119159
AssetSuite::BlobHandle AssetSuite::Internal::RuntimeState::BlobStorage::Create(Blob blob)
120160
{
121-
auto storedBlob = std::make_unique<AssetSuiteBlob_t>(std::move(blob));
122-
BlobHandle handle = storedBlob.get();
123-
blobs.push_back(std::move(storedBlob));
124-
return handle;
161+
size_t slotIndex = 0;
162+
if (!freeSlots.empty())
163+
{
164+
slotIndex = freeSlots.back();
165+
freeSlots.pop_back();
166+
}
167+
else
168+
{
169+
slotIndex = slots.size();
170+
slots.push_back({ nullptr, INITIAL_BLOB_GENERATION });
171+
}
172+
173+
slots[slotIndex].blob = std::make_unique<Blob>(std::move(blob));
174+
return EncodeBlobHandle(slotIndex, slots[slotIndex].generation);
125175
}
126176

127177
bool AssetSuite::Internal::RuntimeState::BlobStorage::Owns(BlobHandle blob) const noexcept
128178
{
129-
for (const auto& storedBlob : blobs)
130-
{
131-
if (storedBlob.get() == blob)
132-
{
133-
return true;
134-
}
135-
}
136-
137-
return false;
179+
return Get(blob) != nullptr;
138180
}
139181

140182
bool AssetSuite::Internal::RuntimeState::BlobStorage::IsLive(BlobHandle blob) const noexcept
141183
{
142-
return Owns(blob) && blob->IsLive();
184+
return Get(blob) != nullptr;
185+
}
186+
187+
const AssetSuite::Internal::Blob*
188+
AssetSuite::Internal::RuntimeState::BlobStorage::Get(BlobHandle blob) const noexcept
189+
{
190+
size_t slotIndex = 0;
191+
uint32_t generation = 0;
192+
if (!DecodeBlobHandle(blob, slotIndex, generation))
193+
{
194+
return nullptr;
195+
}
196+
197+
if (slotIndex >= slots.size())
198+
{
199+
return nullptr;
200+
}
201+
202+
const Slot& slot = slots[slotIndex];
203+
if (slot.generation != generation || !slot.blob)
204+
{
205+
return nullptr;
206+
}
207+
208+
return slot.blob.get();
143209
}
144210

145211
AssetSuite::Result AssetSuite::Internal::RuntimeState::BlobStorage::Release(BlobHandle* blob) noexcept
146212
{
147-
if (!blob || !*blob || !IsLive(*blob))
213+
size_t slotIndex = 0;
214+
uint32_t generation = 0;
215+
if (!blob || !DecodeBlobHandle(*blob, slotIndex, generation))
148216
{
149217
return Result::ErrorInvalidHandle;
150218
}
151219

152-
(*blob)->Release();
220+
if (slotIndex >= slots.size())
221+
{
222+
return Result::ErrorInvalidHandle;
223+
}
224+
225+
Slot& slot = slots[slotIndex];
226+
if (slot.generation != generation || !slot.blob)
227+
{
228+
return Result::ErrorInvalidHandle;
229+
}
230+
231+
slot.blob.reset();
232+
slot.generation = NextBlobGeneration(slot.generation);
233+
freeSlots.push_back(slotIndex);
153234
*blob = nullptr;
154235
return Result::Success;
155236
}
156237

157238
size_t AssetSuite::Internal::RuntimeState::BlobStorage::LiveCount() const noexcept
158239
{
159240
size_t count = 0;
160-
for (const auto& storedBlob : blobs)
241+
for (const auto& slot : slots)
161242
{
162-
if (storedBlob->IsLive())
243+
if (slot.blob)
163244
{
164245
++count;
165246
}
@@ -168,6 +249,11 @@ size_t AssetSuite::Internal::RuntimeState::BlobStorage::LiveCount() const noexce
168249
return count;
169250
}
170251

252+
size_t AssetSuite::Internal::RuntimeState::BlobStorage::SlotCapacity() const noexcept
253+
{
254+
return slots.size();
255+
}
256+
171257
bool AssetSuite::Internal::RuntimeState::CodecRegistry::RegisterImageDecoder(
172258
ImageDecoders decoder,
173259
ImageDecoder& implementation) noexcept

source/runtime/AssetSuiteRuntimeState.h

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,19 +59,28 @@ namespace AssetSuite::Internal
5959

6060
struct FileLoader
6161
{
62-
ErrorCode LoadToMemory(const std::filesystem::path& fileName, bool isBinary, std::vector<BYTE>& output) const;
62+
ErrorCode LoadToMemory(const std::filesystem::path& fileName, bool isBinary, std::vector<uint8_t>& output) const;
6363
};
6464

6565
struct BlobStorage
6666
{
6767
BlobHandle Create(Blob blob);
6868
bool Owns(BlobHandle blob) const noexcept;
6969
bool IsLive(BlobHandle blob) const noexcept;
70+
const Blob* Get(BlobHandle blob) const noexcept;
7071
Result Release(BlobHandle* blob) noexcept;
7172
size_t LiveCount() const noexcept;
73+
size_t SlotCapacity() const noexcept;
7274

7375
private:
74-
std::vector<std::unique_ptr<AssetSuiteBlob_t>> blobs;
76+
struct Slot
77+
{
78+
std::unique_ptr<Blob> blob;
79+
uint32_t generation = 1;
80+
};
81+
82+
std::vector<Slot> slots;
83+
std::vector<size_t> freeSlots;
7584
};
7685

7786
struct CodecRegistry
@@ -133,7 +142,7 @@ namespace AssetSuite::Internal
133142
FileInfo fileInfo;
134143
ImageInfo imageInfo;
135144
MeshInfo meshInfo;
136-
std::vector<BYTE> rawBuffer;
145+
std::vector<uint8_t> rawBuffer;
137146
std::vector<BYTE> decodedBuffer;
138147
std::vector<BYTE> formattedBuffer;
139148
CodecStorage codecs;

source/runtime/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ The public SDK surface is guarded by `PublicHeaderCompile`, `PublicHeaderHygiene
3636

3737
`Internal::Blob` stores raw asset bytes in runtime-owned memory and copies minimal source metadata into the runtime object. The metadata currently tracks the original source path, source extension, and best-known public `AssetFormat` derived from the extension when possible.
3838

39-
`AssetSuiteBlob_t` is the private bridge behind `BlobHandle`. It owns one `Internal::Blob` while the handle is live and remains outside the installed SDK headers.
39+
`BlobHandle` is treated as an opaque slot/generation token by the runtime. The public type remains pointer-shaped for ABI opacity, but blob validation decodes the token and never dereferences the handle value directly.
4040

41-
`RuntimeState::BlobStorage` owns all blob bridge objects for one context. `ReleaseBlob` clears the blob payload and nulls the caller's handle, but the inactive bridge remains in the owning context until context destruction so copied stale handles can be rejected deterministically. Handles from another context are rejected because each context validates against only its own blob storage.
41+
`RuntimeState::BlobStorage` owns reusable blob slots for one context. `ReleaseBlob` clears the blob payload, advances the slot generation, returns the slot to the free list, and nulls the caller's handle. Copied stale handles are rejected by generation mismatch, and handles from another context are rejected because each context validates against only its own slot table.
4242

4343
## Deferred Scope
4444

0 commit comments

Comments
 (0)