blink: Implement stack-allocated IDL unions for hot path arguments [chromium/src : main]
chromeperf@appspot.gserviceaccount.com (Gerrit)
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
😿 Job mac-m1_mini_2020-perf/speedometer3 failed.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/126e8531c90000
Related details
- Anton Bikineev
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
😿 Job mac-m1_mini_2020-perf/speedometer3 failed.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/14c89cfec90000
chromeperf@appspot.gserviceaccount.com (Gerrit)
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job mac-m1_mini_2020-perf/speedometer3 complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/117aba46c90000
Related details
- Anton Bikineev
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job mac-m1_mini_2020-perf/speedometer3 complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/1300ee41c90000
Anton Bikineev (Gerrit)
Anton Bikineev voted and added 1 comment![]( "Open in Gerrit")
Votes added by Anton Bikineev
| Commit-Queue | +1 |
1 comment
Andrey, could you please take a look if the change makes sense? In theory, I'd expect it to visibly improve S3 performance, but, unfortunately, there is no effect. The change should still reduce the GC pressure significantly.
Related details
- Andrey Kosyakov
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrey Kosyakov (Gerrit)
Andrey Kosyakov added 10 comments![]( "Open in Gerrit")
STACK_ALLOCATED() and use raw pointers (T*) instead of Member<T>:So... If we ever need one as a dictionary member, will we have to undo it?
concept IsBlinkNullableType =Move it into `namespace internal` below?
return T::Create(isolate, value, exception_state);So from what I understand the kNone enum variant is so that we have something to return here in case the conversion throws. How about we return `std::optional<>` instead?
from .union import STACK_ALLOCATED_UNIONSLet's expose a predicate method instead? Something like `is_stack_allocated_union(idl_type)`?
return _format("{0}.has_value() ? &*{0} : nullptr", name)`s/{0}/{}/` for consistency?
STACK_ALLOCATED_UNIONS = {I think this rather belongs to blink_v8_bridge.py conceptually as it deals with blink-specific type names.
entries.append("kNone")Not sure I understand why this is needed? Thought we're handling null values via std::optional<>? This also seems to create quite a bit of hassle on the receiving side.
func_decl = CxxFuncDeclNode(name=cg_context.class_name,nit: you can use `func_decl = func_def.make_decl()` for brevity.
if cg_context.class_name in STACK_ALLOCATED_UNIONS and member.type_info.is_gc_type:I assume for stack-allocated unions, the entire Clear() function can be a no-op?
TextNode("STACK_ALLOCATED();\npublic:\n"))just put it into `private_secion`? That "\npublic:\n" part doesn't look nice.
Related details
- Anton Bikineev
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |