FEAT: extend dynamically allocated evidence array by additionalCapacity #59

justadreamer · 2024-10-25T16:37:56Z

in case derived evidence is to be added on the fly during device detection (f.e. derived from 51d_gethighentropyvalues)

justadreamer · 2024-10-28T09:30:27Z

decision is to make an additional array to hold extra evidence, rather than instruct on the call site

… going to be exceeded when new evidence is added a new array is created and linked to the original one. This is needed to avoid situations where GHEV or other dynamic evidence is unpacked and there is insufficient capacity in the original evidence array. The developer would receive unpredicatable results and it would not be obvious where the problem is. The dynamic allocation of more memory for evidence can be avoided by specifying a sufficiently large evidence capacity when the initial evidence array is created. The void pointer used in pairs and evidence pairs has been removed and the KeyValuePair with character pointers used throughout. This reduces problems with casting void pointers for no reason. The evidence key value pair now uses the key value pair to reduce duplication and ensure that all strings have length fields to reduce calls to strlen and to handle GHEV evidence where the string may not be null terminated. Code that expects the keys, values, or parsed value to be null terminated now needs to use the length field to get the number of characters. If a null terminated version is needed then the caller will need to make a copy and add a null terminator.

… this now that evidence can be expanded dynamically.

…red headers and evidence.

justadreamer · 2024-10-29T12:18:10Z

The implementation works for me, there was a minor check added to EvidenceFree. The tests that use this from the DD side are passing.

Two considerations:

Perhaps it would be simpler to use realloc instead of managing linked lists. It can also turn out to be more performant. With realloc there is a high chance of not having to do any new allocations, with linked lists - we are guaranteed to do new allocations. Given evidence arrays are mainly structs with pointers (not raw string buffers) - realloc will extend the already allocated available region of memory and reuse the same pointer and not have to actually allocate and move anything - in that case it is constant time - that should be the behavior in the majority of cases. Move occurs only worst case if a contiguous region of memory is not available in the same memory location - and it has to allocate a new one, but copying in this case would also be quite quick as each element in the evidence array is just several numbers (string pointers and lengths). Strings themselves are not moved. We would need to have FIFTYONE_DEGREES_ARRAY_REALLOC macro and underlying MemoryTrackingRealloc, MemoryStandardRealloc that would call standard realloc inside.
If we don't provide the capacity parameter to init EvidenceBase - we don't let the users of higher-level languages hint the capacity needed for the expanded evidence array, so by default it will always be creating tight evidence arrays and we the underlying code will always have to add capacity either through linked lists or realloc. Perhaps that feature may still be provided for some use cases requiring high performance to not have reallocate, however with realloc it likely won't be needed...

jwrosewell · 2024-10-29T14:56:55Z

Re: Realloc - It is more important to make the feature available than add new memory management code for a fringe case. We can consider realloc support in version 5.
Re: EvidenceBase - a new method could be added which provides the dataset that the evidence will be used with so that the dataset can be used to inform the capacity of the C evidence instance returned. We can consider this if we see issues with higher level languages. Generally the overhead of marshaling the data is likely to be greater than using a linked list of lists.

github-actions · 2024-10-30T12:04:39Z

Unit Tests - Mac_x64_Debug

518 tests 518 ✅ 6s ⏱️
1 suites 0 💤
1 files 0 ❌

Results for commit 071b5a7.

github-actions · 2024-10-30T12:04:43Z

Unit Tests - Ubuntu_x64_Debug

520 tests 520 ✅ 4s ⏱️
1 suites 0 💤
1 files 0 ❌

Results for commit 071b5a7.

github-actions · 2024-10-30T12:04:49Z

Integration Tests - Mac_x64_Debug

0 tests 0 ✅ 0s ⏱️
1 suites 0 💤
1 files 0 ❌

Results for commit 071b5a7.

github-actions · 2024-10-30T12:04:50Z

Unit Tests - Ubuntu_x86_Debug

520 tests 520 ✅ 5s ⏱️
1 suites 0 💤
1 files 0 ❌

Results for commit 071b5a7.

github-actions · 2024-10-30T12:04:52Z

Integration Tests - Ubuntu_x64_Debug

0 tests 0 ✅ 0s ⏱️
1 suites 0 💤
1 files 0 ❌

Results for commit 071b5a7.

github-actions · 2024-10-30T12:04:57Z

Unit Tests - Ubuntu_x86_Release

520 tests 520 ✅ 2s ⏱️
1 suites 0 💤
1 files 0 ❌

Results for commit 071b5a7.

github-actions · 2024-10-30T12:04:59Z

Integration Tests - Ubuntu_x86_Debug

0 tests 0 ✅ 0s ⏱️
1 suites 0 💤
1 files 0 ❌

Results for commit 071b5a7.

github-actions · 2024-10-30T12:05:05Z

Integration Tests - Ubuntu_x86_Release

0 tests 0 ✅ 0s ⏱️
1 suites 0 💤
1 files 0 ❌

Results for commit 071b5a7.

github-actions · 2024-10-30T12:05:11Z

Unit Tests - Ubuntu_x64_Release

520 tests 520 ✅ 2s ⏱️
1 suites 0 💤
1 files 0 ❌

Results for commit 071b5a7.

github-actions · 2024-10-30T12:05:14Z

Performance Tests - Ubuntu_x86_Release

0 tests 0 ✅ 0s ⏱️
1 suites 0 💤
1 files 0 ❌

Results for commit 071b5a7.

github-actions · 2024-10-30T12:05:22Z

Integration Tests - Ubuntu_x64_Release

0 tests 0 ✅ 0s ⏱️
1 suites 0 💤
1 files 0 ❌

Results for commit 071b5a7.

github-actions · 2024-10-30T12:05:33Z

Performance Tests - Ubuntu_x64_Release

0 tests 0 ✅ 0s ⏱️
1 suites 0 💤
1 files 0 ❌

Results for commit 071b5a7.

github-actions · 2024-10-30T12:05:58Z