Closed Bug 741222 Opened 8 years ago Closed 8 years ago

[skiplist] Add missing Android libraries to the ignore skip list

Categories

(Socorro :: Infra, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: scoobidiver, Assigned: laura)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [qa-])

Please add:
* 'dalvik-LinearAlloc'
* 'dalvik-jit-code-cache'
* 'dalvik-mark-stack'
* 'ashmem'
* 'zero \(deleted\)@0x.*'
* 'app_process@0x.*'
* 'nvmap@0x.*'
* 'framework\.odex@0x.*'
* 'core\.odex@0x.*'
* 'system@framework@framework\.jar@classes\.dex@0x.*'
* 'system@framework@core\.jar@classes\.dex@0x.*'
* 'system@framework@services\.jar@classes\.dex@0x.*'
* 'data@app@org\.mozilla\.fennec-2\.apk@classes\.dex@0x.*'
* 'mnt@asec@org\.mozilla\.fennec-1@pkg\.apk@classes\.dex@0x.*'
* 'mnt@asec@org\.mozilla\.fennec-2@pkg\.apk@classes\.dex@0x.*'
* 'org\.mozilla\.fennec-1\.apk@0x.*'
* 'org\.mozilla\.fennec-2\.apk@0x.*'
* 'libz\.so@0x.*'
* 'libgui\.so@0x.*'
* 'libMali\.so@0x.*'
* 'libutils\.so@0x.*'
* 'libbinder\.so@0x.*'
* 'libandroid_runtime\.so@0x.*'
Remove:
* 'dalvik-LinearAlloc\s*@0x.*'
Chris, Naoki, do we really want all these on the skiplist?
Scoobi, could you list the signature tags for these crashes to help justify them being on the skiplist please?

Note:
'dalvik-LinearAlloc\s*@0x.*' is suppose to cover dalvik-LinearAlloc (deleted)@0x23b6ee from https://crash-stats.mozilla.com/report/list?range_value=7&range_unit=days&date=2012-04-02&signature=dalvik-LinearAlloc%20%28deleted%29%400x23b6ee&version=FennecAndroid%3A14.0a1

'dalvik-LinearAlloc' does not cover this scenario.  Kairo, would 'dalvik-LinearAlloc*'?  I think it would but not sure.
Also to note, if we have crashes that don't match up with a symbol table, I think the main concern is having Soccoro come back with saying that it doesn't see a crash signature at all.
(In reply to Naoki Hirata :nhirata from comment #2)
> Scoobi, could you list the signature tags for these crashes to help justify
> them being on the skiplist please?
What do you mean by signature tags? Do you want current bugs that depends on that?
The root of this bug is the same as bug 726385: to have one signature with many reports tracked in Socorro with its right ranking instead of many signatures with few reports not reported in Socorro.

(In reply to Naoki Hirata :nhirata from comment #3)
> Also to note, if we have crashes that don't match up with a symbol table, I
> think the main concern is having Soccoro come back with saying that it
> doesn't see a crash signature at all.
The fallback mechanism is to show the first frame as it was not in the ignore skiplist. See bp-276cd543-eae7-480c-8dab-28d8f2120328.
I'm not familiar with the skiplist syntax, but these function names look like reasonable additions for the Fennec skiplist.
This list and the one in bug 741315 look so long and scary for me, I wonder if we really need all of those on the skiplist and about the concrete examples we need to add them for.
Without lib*.so that can be let as it is now, it concerns 21 crash signatures amongst 205 ones (10%) and 25 crashes amongst 455 crashes (6%) over the last 3 days.
Blocks: 745243, 750373
Blocks: 705641
No longer blocks: 705614
Blocks: 749917
Blocks: 749687
No longer blocks: 749687
Blocks: 749687
For instance, there are 119 dalvik-LinearAlloc (deleted) crashes in 14.0b2. Thus bug 745243 which is classified as #3 top crasher with combined signatures is probably #1 top crasher.
Blocks: 757303
Blocks: 758898
Without lib*.so, in 14.0b3, we are blind for 132 crash signatures, ie 17%, and 190 crashes, ie 5.6%. For instance, bug 744850 which is #1 top crasher represents 9.6% of crashes.
This shows the skiplist:
https://github.com/mozilla/socorro/blob/master/scripts/config/processorconfig.py.dist

These are already added : 
'dalvik-LinearAlloc'
'data@app@org\.mozilla\.fennec-1\.apk@classes\.dex@0x.*'

The rest still need to be added, still need to have a line removed:'dalvik-LinearAlloc\s*@0x.*'
(In reply to Scoobidiver from comment #9)
> Without lib*.so, in 14.0b3, we are blind for 132 crash signatures, ie 17%,
> and 190 crashes, ie 5.6%. For instance, bug 744850 which is #1 top crasher
> represents 9.6% of crashes.

Esp. the the lib*.so ones look for me like something that should never be on the ignore list, only possibly on the prefix list. Why do we want to completely omit the place where the actual crash is happening?

(In reply to Chris Peterson (:cpeterson) from comment #5)
> I'm not familiar with the skiplist syntax, but these function names look
> like reasonable additions for the Fennec skiplist.

Does that mean you think it's OK to completely ignore those frames in a crash signature and not show them at all?
(In reply to Naoki Hirata :nhirata from comment #10)
> This shows the skiplist:
> https://github.com/mozilla/socorro/blob/master/scripts/config/
> processorconfig.py.dist
> 
> These are already added : 
> 'dalvik-LinearAlloc'

I don't see this one there.

> 'data@app@org\.mozilla\.fennec-1\.apk@classes\.dex@0x.*'

Right, it's there now, but not the other .apk variants, I wonder why we did it that way.

> The rest still need to be added, 

Are we sure those should all be completely ignored for signatures?

>still need to have a line
> removed:'dalvik-LinearAlloc\s*@0x.*'

I still wonder why we should remove that from the ignore list.
(In reply to Scoobidiver from comment #9)
> Without lib*.so, in 14.0b3, we are blind for 132 crash signatures, ie 17%,
> and 190 crashes, ie 5.6%. For instance, bug 744850 which is #1 top crasher
> represents 9.6% of crashes.

Scoobidiver, can you point to an example of why lib*.so should be on the skiplist? I'm not sure why we would want to skip all .so files. The example bug 744850 you gave is a Java crash that should not be affected by skipping lib*.so.


(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #11)
> (In reply to Chris Peterson (:cpeterson) from comment #5)
> > I'm not familiar with the skiplist syntax, but these function names look
> > like reasonable additions for the Fennec skiplist.
> 
> Does that mean you think it's OK to completely ignore those frames in a
> crash signature and not show them at all?

I don't see a big problem with ignoring the frames matched by the skiplist rules in comment 1 (except the new proposal to skip all lib*.so) when generating a crash signature.
Maybe I wasn't clear in comment 7 and comment 9, but I am fine not to add the last six lines of comment 0.

(In reply to Naoki Hirata :nhirata from comment #10)
> These are already added : 
> 'dalvik-LinearAlloc'
dalvik-LinearAlloc\s*@0x.* is in the ignore skiplist while dalvik-LinearAlloc is required. See https://crash-stats.mozilla.com/query/query?product=FennecAndroid&version=ALL%3AALL&range_value=1&range_unit=weeks&query_search=signature&query_type=contains&query=dalvik-LinearAlloc&reason=&do_query=1

> 'data@app@org\.mozilla\.fennec-1\.apk@classes\.dex@0x.*'
I haven't asked to add this one.
dalvik-LinearAlloc\s*@0x.* should be covering the same query as https://crash-stats.mozilla.com/query/query?product=FennecAndroid&version=ALL%3AALL&range_value=1&range_unit=weeks&query_search=signature&query_type=contains&query=dalvik-LinearAlloc&reason=&do_query=1

I would think that's a bug in Socorro if it's not.  So thinking about things : 'dalvik-LinearAlloc\s*@0x.*' should not be removed, and 'dalvik-LinearAlloc' should not be added.  We should investigate why the regex 'dalvik-LinearAlloc\s*@0x.*' skiplist isn't working because that string should cover the query you mentioned.

There are some skiplist regex magic we can work on to clean the list down some I think before we place some of these in place.  (ie some of the items can be reg exed down)

We should also take a look at the stacks and evaluate them a little bit to see how useful skipping the frame would be.  I believe if the stacks do not have any associate symbols, I am unsure of how helpful skipping the frame would be.
If you guys can reach agreement today, I can land this today and it will go out next week.
Assignee: nobody → laura
Target Milestone: --- → 12
(In reply to Naoki Hirata :nhirata from comment #15)
> I would think that's a bug in Socorro if it's not.  So thinking about things
> : 'dalvik-LinearAlloc\s*@0x.*' should not be removed, and
> 'dalvik-LinearAlloc' should not be added.  We should investigate why the
> regex 'dalvik-LinearAlloc\s*@0x.*' skiplist isn't working because that
> string should cover the query you mentioned.
That what was done in bug 702657. You can investigate latter.

> There are some skiplist regex magic we can work on to clean the list down
> some I think before we place some of these in place.  (ie some of the items
> can be reg exed down)
Can you optimize my regexp expressions?

> We should also take a look at the stacks and evaluate them a little bit to
> see how useful skipping the frame would be.  I believe if the stacks do not
> have any associate symbols, I am unsure of how helpful skipping the frame
> would be.
I did it before filing this bug. Sometimes, the fallback mechanism (first frame for signature) will be used, but other times, it will be useful. Here are some examples
bp-2c2ebe6f-e719-42fa-8f55-267122120530 -> nsCOMPtr_base::~nsCOMPtr_base | nsStringBuffer::Release
bp-2e3b7fdd-4c87-4387-8acd-c15152120531 -> _JNIEnv::CallStaticVoidMethod (bug 758898)
bp-2daf21f8-8394-4f02-a2f6-d493d2120530 -> arena_run_split
bp-b01f2356-4fcd-4acd-a16d-57c4c2120530 -> _JNIEnv::CallObjectMethod | getSurface
bp-a0763aac-7085-4779-beaa-3893a2120529 -> TouchBadMemory | mozalloc_abort | XPCConvert::JSData2Native
bp-1cc89333-9476-4f06-946f-32d112120529 -> TouchBadMemory | mozalloc_abort | nsACString_internal::AppendFunc
bp-803f6996-1f54-4752-b7e2-79a662120530 -> _Znwj
bp-f5a0b0ed-61cb-4527-af42-93b172120530 -> dlmalloc_walk_free_pages | __thread_entry
bp-2189628f-c3fd-426f-9cfd-0524c2120530 -> libGLESv2_tegra.so@0x8386
bp-cec2dd91-6c27-4d22-8128-f85922120531 -> libGLESv2_tegra.so@0x8386
Fair point on both accounts.  So the only work that needs to be done is to optimize the regex as far as I'm concerned then.  Kairo can you double check these please?

* 'dalvik-LinearAlloc'
* 'dalvik-jit-code-cache'
* 'dalvik-mark-stack'
* 'ashmem'
* 'zero'
* 'app_process@0x.*'
* 'nvmap@0x.*'
* 'framework\.odex@0x.*'
* 'core\.odex@0x.*'
* 'libz\.so@0x.*'
* 'libgui\.so@0x.*'
* 'libMali\.so@0x.*'
* 'libutils\.so@0x.*'
* 'libbinder\.so@0x.*'
* 'libandroid_runtime\.so@0x.*'

* 'system@framework@*\.jar@classes\.dex@0x.*'

* 'data@app@org\.mozilla\.fennec-\d\.apk@classes\.dex@0x.*'
* 'mnt@asec@org\.mozilla\.fennec-\d@pkg\.apk@classes\.dex@0x.*'

* 'org\.mozilla\.fennec-\d\.apk@0x.*'

Remove:
* 'dalvik-LinearAlloc\s*@0x.*'
* 'data@app@org\.mozilla\.fennec-1\.apk@classes\.dex@0x.*'

Note:
The last 4 additions should cover:
= 'system@framework@framework\.jar@classes\.dex@0x.*'
= 'system@framework@core\.jar@classes\.dex@0x.*'
= 'system@framework@services\.jar@classes\.dex@0x.*'

= 'data@app@org\.mozilla\.fennec-1\.apk@classes\.dex@0x.*'
= 'data@app@org\.mozilla\.fennec-2\.apk@classes\.dex@0x.*'

= 'mnt@asec@org\.mozilla\.fennec-1@pkg\.apk@classes\.dex@0x.*'
= 'mnt@asec@org\.mozilla\.fennec-2@pkg\.apk@classes\.dex@0x.*'

= 'org\.mozilla\.fennec-1\.apk@0x.*'
= 'org\.mozilla\.fennec-2\.apk@0x.*'
Those look good to me, but we should watch closely as e.g. adding the graphics driver libMali.so might actually end up hiding crashes in the driver and attributing them to the caller. I guess we should just do them and QA for doing the right thing there.
Great.  I'll follow up once this is implemented to verify and make sure that the right thing is done (ie address the concern in comment 19).
Blocks: 748499
Blocks: 759162
Commit pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/9ec5574e0829b6f7bdd6e14116d162fb48a2382c
Fixes bug 741222, add missing android libraries to the irrelevant
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Commit pushed to stage at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/2f4773b949eca92bde67507261595f52f9a6db64
Fixes bug 741222, add missing android libraries to the irrelevant
signature list
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.