dawn_wire: Directly use the data in deserialized buffer when possible
This patch tries to use the data directly in deserialized buffer when the data is "data-only" and doesn't affect the control flow instead of allocating and copying into a temporary buffer. Due to the protection of TOCTOU attacks, currently we only set "wire_is_data_only" on the parameter "data" in Queue.WriteBuffer() and Queue.WriteTexture(). With this patch, the performance of dawn_perf_tests BufferUploadPerf.Run/*_WriteBuffer_BufferSize_* with "-w" will be greatly improved (~20%) when the upload buffer size is greater than 1MB. BUG=chromium:1266727 Change-Id: I7a9d54c9b505975235ee37aa72ee97f082ad3aa3 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/72063 Reviewed-by: Austin Eng <enga@chromium.org> Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
This commit is contained in:
parent
6ad6278bc5
commit
1f25ea94bb
|
@ -70,13 +70,13 @@
|
||||||
{"name": "queue id", "type": "ObjectId" },
|
{"name": "queue id", "type": "ObjectId" },
|
||||||
{"name": "buffer id", "type": "ObjectId" },
|
{"name": "buffer id", "type": "ObjectId" },
|
||||||
{"name": "buffer offset", "type": "uint64_t"},
|
{"name": "buffer offset", "type": "uint64_t"},
|
||||||
{"name": "data", "type": "uint8_t", "annotation": "const*", "length": "size"},
|
{"name": "data", "type": "uint8_t", "annotation": "const*", "length": "size", "wire_is_data_only": true},
|
||||||
{"name": "size", "type": "uint64_t"}
|
{"name": "size", "type": "uint64_t"}
|
||||||
],
|
],
|
||||||
"queue write texture": [
|
"queue write texture": [
|
||||||
{"name": "queue id", "type": "ObjectId" },
|
{"name": "queue id", "type": "ObjectId" },
|
||||||
{"name": "destination", "type": "image copy texture", "annotation": "const*"},
|
{"name": "destination", "type": "image copy texture", "annotation": "const*"},
|
||||||
{"name": "data", "type": "uint8_t", "annotation": "const*", "length": "data size"},
|
{"name": "data", "type": "uint8_t", "annotation": "const*", "length": "data size", "wire_is_data_only": true},
|
||||||
{"name": "data size", "type": "uint64_t"},
|
{"name": "data size", "type": "uint64_t"},
|
||||||
{"name": "data layout", "type": "texture data layout", "annotation": "const*"},
|
{"name": "data layout", "type": "texture data layout", "annotation": "const*"},
|
||||||
{"name": "writeSize", "type": "extent 3D", "annotation": "const*"}
|
{"name": "writeSize", "type": "extent 3D", "annotation": "const*"}
|
||||||
|
|
|
@ -42,6 +42,7 @@ A **record** is a list of **record members**, each of which is a dictionary with
|
||||||
- `"length"` (default to 1 if not set), a string. Defines length of the array pointed to for pointer arguments. If not set the length is implicitly 1 (so not an array), but otherwise it can be set to the name of another member in the same record that will contain the length of the array (this is heavily used in the `fooCount` `foos` pattern in the API). As a special case `"strlen"` can be used for `const char*` record members to denote that the length should be determined with `strlen`.
|
- `"length"` (default to 1 if not set), a string. Defines length of the array pointed to for pointer arguments. If not set the length is implicitly 1 (so not an array), but otherwise it can be set to the name of another member in the same record that will contain the length of the array (this is heavily used in the `fooCount` `foos` pattern in the API). As a special case `"strlen"` can be used for `const char*` record members to denote that the length should be determined with `strlen`.
|
||||||
- `"optional"` (default to false) a boolean that says whether this member is optional. Member records can be optional if they are pointers (otherwise dawn_wire will always try to dereference them), objects (otherwise dawn_wire will always try to encode their ID and crash), or if they have a `"default"` key. Optional pointers and objects will always default to `nullptr`.
|
- `"optional"` (default to false) a boolean that says whether this member is optional. Member records can be optional if they are pointers (otherwise dawn_wire will always try to dereference them), objects (otherwise dawn_wire will always try to encode their ID and crash), or if they have a `"default"` key. Optional pointers and objects will always default to `nullptr`.
|
||||||
- `"default"` (optional) a number or string. If set the record member will use that value as default value. Depending on the member's category it can be a number, a string containing a number, or the name of an enum/bitmask value.
|
- `"default"` (optional) a number or string. If set the record member will use that value as default value. Depending on the member's category it can be a number, a string containing a number, or the name of an enum/bitmask value.
|
||||||
|
- `"wire_is_data_only"` (default to false) a boolean that says whether it is safe to directly return a pointer of this member that is pointing to a piece of memory in the transfer buffer into dawn_wire. To prevent TOCTOU attacks, by default in dawn_wire we must ensure every single value returned to dawn_native a copy of what's in the wire, so `"wire_is_data_only"` is set to true only when the member is data-only and don't impact control flow.
|
||||||
|
|
||||||
**`"native"`**, doesn't have any other key. This is used to define native types that can be referenced by name in other things.
|
**`"native"`**, doesn't have any other key. This is used to define native types that can be referenced by name in other things.
|
||||||
|
|
||||||
|
|
|
@ -373,27 +373,40 @@
|
||||||
const volatile {{member_transfer_type(member)}}* memberBuffer;
|
const volatile {{member_transfer_type(member)}}* memberBuffer;
|
||||||
WIRE_TRY(deserializeBuffer->ReadN(memberLength, &memberBuffer));
|
WIRE_TRY(deserializeBuffer->ReadN(memberLength, &memberBuffer));
|
||||||
|
|
||||||
|
//* For data-only members (e.g. "data" in WriteBuffer and WriteTexture), they are
|
||||||
|
//* not security sensitive so we can directly refer the data inside the transfer
|
||||||
|
//* buffer in dawn_native. For other members, as prevention of TOCTOU attacks is an
|
||||||
|
//* important feature of the wire, we must make sure every single value returned to
|
||||||
|
//* dawn_native must be a copy of what's in the wire.
|
||||||
|
{% if member.json_data["wire_is_data_only"] %}
|
||||||
|
record->{{memberName}} =
|
||||||
|
const_cast<const {{member_transfer_type(member)}}*>(memberBuffer);
|
||||||
|
|
||||||
|
{% else %}
|
||||||
{{as_cType(member.type.name)}}* copiedMembers;
|
{{as_cType(member.type.name)}}* copiedMembers;
|
||||||
WIRE_TRY(GetSpace(allocator, memberLength, &copiedMembers));
|
WIRE_TRY(GetSpace(allocator, memberLength, &copiedMembers));
|
||||||
record->{{memberName}} = copiedMembers;
|
record->{{memberName}} = copiedMembers;
|
||||||
|
|
||||||
{% if member.type.is_wire_transparent %}
|
{% if member.type.is_wire_transparent %}
|
||||||
//* memcpy is not allowed to copy from volatile objects. However, these arrays
|
//* memcpy is not allowed to copy from volatile objects. However, these
|
||||||
//* are just used as plain data, and don't impact control flow. So if the
|
//* arrays are just used as plain data, and don't impact control flow. So if
|
||||||
//* underlying data were changed while the copy was still executing, we would
|
//* the underlying data were changed while the copy was still executing, we
|
||||||
//* get different data - but it wouldn't cause unexpected downstream effects.
|
//* would get different data - but it wouldn't cause unexpected downstream
|
||||||
|
//* effects.
|
||||||
memcpy(
|
memcpy(
|
||||||
copiedMembers,
|
copiedMembers,
|
||||||
const_cast<const {{member_transfer_type(member)}}*>(memberBuffer),
|
const_cast<const {{member_transfer_type(member)}}*>(memberBuffer),
|
||||||
{{member_transfer_sizeof(member)}} * memberLength);
|
{{member_transfer_sizeof(member)}} * memberLength);
|
||||||
{% else %}
|
{% else %}
|
||||||
//* This loop cannot overflow because it iterates up to |memberLength|. Even if
|
//* This loop cannot overflow because it iterates up to |memberLength|. Even
|
||||||
//* memberLength were the maximum integer value, |i| would become equal to it
|
//* if memberLength were the maximum integer value, |i| would become equal
|
||||||
//* just before exiting the loop, but not increment past or wrap around.
|
//* to it just before exiting the loop, but not increment past or wrap
|
||||||
|
//* around.
|
||||||
for (decltype(memberLength) i = 0; i < memberLength; ++i) {
|
for (decltype(memberLength) i = 0; i < memberLength; ++i) {
|
||||||
{{deserialize_member(member, "memberBuffer[i]", "copiedMembers[i]")}}
|
{{deserialize_member(member, "memberBuffer[i]", "copiedMembers[i]")}}
|
||||||
}
|
}
|
||||||
{% endif %}
|
{% endif %}
|
||||||
|
{% endif %}
|
||||||
}
|
}
|
||||||
{% endfor %}
|
{% endfor %}
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue