999126 - Add exidx directives to bionic's memcpy

Status: NEW

(Core :: Graphics, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

Attachment: Add exidx directives to bionic's memcpy

Comment 1

[Benoit Girard (:BenWa)]

Here's a profile where this would of been useful:
https://people.mozilla.org/~bgirard/cleopatra/#report=25f3082c62a4a74ad7e65eb70a6e9fe0cae8f9ce

Comment 2

[Benoit Girard (:BenWa)]

Comment on attachment 8409828 [details] [diff] [review]
Add exidx directives to bionic's memcpy

Do you know the right person to get this change upstreamed? It would be useful for performance analysis.

Comment 3

[Diego Wilson [:diego]]

What exactly does this patch achieve?

Comment 4

[Andreas Gal :gal]

This is adding unwinding information, presumably for better profiles.

Comment 5

[Benoit Girard (:BenWa)]

Yes that correct. If we're hitting code through this memcpy implementation we wont be able to unwind and find what is requesting the memcpy. This is important if we're seeing an unreasonable amount of memcpy time like we were seeing a canvas profile to investigate why.

This should just add to the unwind tables which are typically not mapped AFAIK. They shouldn't impact the memcpy performance since they wont change the memcpy function itself.

Comment 6

[Diego Wilson [:diego]]

Sounds good, I'll see what I can do to pull it to CAF

Comment 7

[Diego Wilson [:diego]]

Benwa,

Sorry it took me so long to get around to this one. The baseline of your patch looks very different than CAF's tip. This is what we're currently using:

https://www.codeaurora.org/cgit/quic/la/platform/bionic/commit/?id=e36fba720d5327bf5532dfcde013239080f38b56

Comment 8

[Benoit Girard (:BenWa)]

Looks like it does the right thing but uses cfi_startproc instead of fnstart.

Comment 9

[Jeff Muizelaar [:jrmuizel]]

Can we land this on the b2g/ics_strawberry branch?

Comment 10

[Diego Wilson [:diego]]

The ICS Gonk on CAF is very solid right now. I doubt anything short of a critical security patch would land there.

Comment 11

[Diego Wilson [:diego]]

So if anything, this would land on Kitkat Gonk on CAF.