Closed Bug 502820 Opened 16 years ago Closed 8 years ago

bug id harvesting should match signatures with just [@ Foo] instead of requiring [@ Foo(...)]

Categories

(Socorro :: General, task)

Product:
Socorro
Component:
General
Platform:
x86
macOS
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: timeless, Assigned: lonnen)

References

Details

having full arguments in bug summaries reduces the ability to have a bunch of signatures in summaries and is rarely if ever beneficial, please recognize the shorter syntax always.
WSM asked lars about this on IRC yesterday, and lars noted that the reason it's implemented as it is is because the full signature is an indexed column, whereas doing a partial match would require table scanning, which kills the db. That being said, I think we could implement this by adding another column to the reports table which contained the signature minus the argument list.
(In reply to comment #1) > That being said, I think we could implement this by adding another column to > the reports table which contained the signature minus the argument list. would adding that column also potentially allow search "exactly" to find signatures without the arguments?
Doing this is a vote in favor of (re)building signaturedims table
Just a reminder that functions in C++ are overloaded on parameter type (which is what I think wsmwk means by 'arguments') so "search 'exactly'" without the parameter types might sometimes find overload function sets rather than a specific function.
What would be the purpose of maintaining two "overall signature" columns? Wouldn't it be better to just change how signatures are created for the existing column? I don't see the benefit of having one for display and one for associations. The purpose in C++ for including the parameters in the signature is to disambiguate overloaded functions. By ignoring parameters, are we introducing ambiguity that will cause problems in the future? What about template parameters? There are a number of other 'inconvenient' syntax artifacts in signatures with square brackets, angle brackets, braces, back tics and apostrophes. Should they be simplified too? Examples: nsTHashtable<nsBaseHashtableET<nsISupportsHashKey, nsCOMPtr<nsIXULTemplateBuilder> > >::s_MatchEntry(PLDHashTab le*, PLDHashEntryHdr const*, void const*) [thunk]:ATL::CComContainedObject<CWMPButtonCtrl>::Release`adjustor{12}' () I think this bug is really a call for a reexamination on how signatures are normalized.
Template information is usually important. nsTHashtable...nsIXULTemplateBuilder is going to be a very different bug than the same method with some other interface. I'm not sure why we can't just use the long signatures... are we actually running up against length limits in the whiteboard/summary field?
When I talked with Lars about this, I didn't see any harm including the entire signature in the summary field. I still don't. Unless someone has a good reason to change (and I haven't seen one yet), I don't think we should add the complication to Socorro.
I think timeless' point is just that historically people have not used the full signatures in bug summaries. I sort of agree that full argument lists don't add much value in bug summaries, and I doubt there would be much confusion with overloaded methods.
we do run into limits with the summary field, and it makes summaries more painful. Supporting both syntaxes is ideal, people can certainly file bugs w/ template signatures, in those cases, they'll know if they think they're relevant. Frank: in practice we rarely have cases where overloads are a problem, and again, if it actually is relevant someone can change their bug summary to have the full signature.
IMO "Supporting both syntaxes is ideal". Question - If parameters are so useful in bug summaries, wouldn't they be more prevalent? Summaries of open crash bugs: * FF has 600+ without parameters, and 100 with * TB has ~80 without parameters and 20 with
Question: How does a signature get into a bug? If we are doing it by hand, then cut and paste seems obvious and whatever is easily cut should be used (and we should think about how to automate it). If somehow automagically, then the automaton and the database should work together. (In reply to comment #9 and comment #4): If you know that function overload sets are unlikely to be a problem, I'm perfectly happy to agree with you. But since some connections are automatic now, it is probably better in general to use the full signature to avoid unintended mis-connections: If a problem is rare but real, then people tend to forget how to deal with it, and the cost of (re) discovering the problem each time is high.
It is indeed by hand. I think Wayne's point is that if you look in bugzilla, the vast majority of existing bugs with signatures in the summary do not contain the parameter list.
(In reply to comment #12) > I think Wayne's point is that if you look in bugzilla, > the vast majority of existing bugs with signatures in the summary do not > contain the parameter list. Correct. That fact leads to these points: a) bug summaries seem to have been sufficient to date without parameters, so a1) why has it been sufficient? and a2) does it continue to be sufficient? b) if either choice is not a slam dunk, do we consider the inertia of what people have been accustomed to doing? c) if there is reason to change, surely we aren't going to change ~700 old bugs, so then we still would want to bridge matching these to old bugs crash-stats
(In reply to comment #9) > we do run into limits with the summary field, and it makes summaries more > painful. > nsObjCExceptionLogAbort | nsNativeThemeCocoa::DrawWidgetBackground(nsIRenderingContext*, nsIFrame*, unsigned char, nsRect const&, nsRect const&) is more characters than a normal bug summary by itself (145) and is therefore an awful lot to put in a bug summary in order to get Bugzilla integration (150 when you add [@ ]). [@ nsObjCExceptionLogAbort | nsNativeThemeCocoa::DrawWidgetBackground] is still a bit long, but far more manageable; you could still manage to fit a "why" before or after and maybe have a chance of seeing it, or a chunk of the signature, in bug lists.
If we're not going to fix this, then we really need to train developers hard to copy/paste the whole stupid signature from the crash report H1 when filing bugs. I maintain it's far easier to teach the software to work the way everyone expects it to work than to retrain developers to work the way the software currently wants developers to work ;-) (Also inserting "signature" into the summary so this can be found more easily.)
Summary: bug id harvesting should recognize [@ Foo] instead of requiring [@ Foo(...)] → bug id harvesting should match signatures with just [@ Foo] instead of requiring [@ Foo(...)]
(In reply to comment #6) > I'm not sure why we can't just use the long signatures... are we actually > running up against length limits in the whiteboard/summary field? Yes; I just did while trying to put three signatures in the summary of bug 521844; I gave up and put only the two shorter ones.
I also hit summary length limits a couple of times last weekend when I was filing new bugs based on crash-stats reports :(
In bug 541743, I wanted to add the "plain" [@ nsDocShell::Destroy() ] signature to the summary, but I'm out of room. There's enough characters in the arguments to nsCOMPtr_base::assign_from_qi to allow me to do that, if only we'd match on [@ Foo ] instead of [@ Foo(…) ].
Component: Socorro → General
Product: Webtools → Socorro
This was fixed by: https://bugzilla.mozilla.org/show_bug.cgi?id=634276
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
(In reply to Chris Lonnen :lonnen from comment #20) > This was fixed by: https://bugzilla.mozilla.org/show_bug.cgi?id=634276 Not really. We don't match Foo when Foo(…) is the signature, AFAIK. But not sure if we really need to do that given that the Crash Signature field allows multiple entries more easily.
My misunderstanding. Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Lonnen: did rewriting the cron job in Q1 address this? If yes please mark RESOLVED.
Assignee: nobody → chris.lonnen
Flags: needinfo?(chris.lonnen)
Not at all. Is this still desired?
Flags: needinfo?(chris.lonnen)
I want it because I often paste signatures from local debuggers, and I don't know if they all show parameters the same way as crash-stats. (There's also the problem that different functions might be inlined, of course.)
We are unlikely to pursue this further. If this is still desired, please reopen.
Status: REOPENED → RESOLVED
Closed: 13 years ago → 8 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.