IndexedDB: Each place of failure should have its own histogram entry
Created attachment 176853 [details] Patch
Josh and/or Alec, could you take a look at this? Given how little we know about what's happening, I want to go with this brute force approach instead of trying to be too intelligent. Counterproposals welcome of course.
Michael did something similar with WebSQL callsites. His was more structured though, he broke them out by operation type, and then by callsite. http://go/michaelswebsqlstats
Created attachment 177007 [details] Patch updated to ToT
Tony, could you review this? After some uncertainty we (jsbell+alecflett+me) agreed in person that this approach was fine. Your thoughts are also welcome.
Each value you add takes additional memory, so trying to keep the number of values small will help a bit. I'm not sure if you plan on reusing/renumbering values or just keep adding. It seems like it would be confusing as the code changes to keep track of what values are needed. So while this works, I think it would be a bit better if instead of just numbering the errors, you named them and perhaps grouped them. For example, you could have a distinct error for each function and name the enum values to match the function names. This seems easier to maintain going forward. On the other hand, maybe you plan on removing this after one or two dev channel release, in which case, maybe it doesn't matter.
Created attachment 177340 [details] Patch
This patch switches to the one-enum-entry-per-function approach. Tony, is this what you had in mind?
Comment on attachment 177340 [details] Patch Yes, that's what I had in mind.
just coming late to this, (maybe too late) but how do the additional values use more memory? I think we can safely assume that these enum values are just going to get hardcoded into the compiled instructions, so there shouldn't be any difference between movl eax, 0x12 and movl eax, 0x23
Comment on attachment 177340 [details] Patch Rejecting attachment 177340 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ripts/update-webkit line 152. Failed to run "['Tools/Scripts/update-webkit', '--chromium', '--force-update']" exit_code: 2 Updating OpenSource From http://git.chromium.org/external/Webkit 9961a7b..ef0ca77 HEAD -> origin/HEAD error: Ref refs/remotes/origin/master is at ef0ca77a554ed2a07b001ee4935266d6301b2bf8 but expected 9961a7b754078ea893b086fbc787c57745c8a4db ! 9961a7b..ef0ca77 master -> origin/master (unable to update local ref) Died at Tools/Scripts/update-webkit line 152. Full output: http://queues.webkit.org/results/15117748
Comment on attachment 177340 [details] Patch Rejecting attachment 177340 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ripts/update-webkit line 152. Failed to run "['Tools/Scripts/update-webkit', '--chromium', '--force-update']" exit_code: 2 Updating OpenSource From http://git.chromium.org/external/Webkit 54297d7..837f7c4 HEAD -> origin/HEAD error: Ref refs/remotes/origin/master is at 837f7c4da6fc1cce27d8517a8319c0e408da63da but expected 54297d74214132a39292f05bb9466fdab588b0aa ! 54297d7..837f7c4 master -> origin/master (unable to update local ref) Died at Tools/Scripts/update-webkit line 152. Full output: http://queues.webkit.org/results/15121656
Comment on attachment 177340 [details] Patch Clearing flags on attachment: 177340 Committed r136497: <http://trac.webkit.org/changeset/136497>
All reviewed patches have been landed. Closing bug.
(In reply to comment #10) > just coming late to this, (maybe too late) but how do the additional values use more memory? I think we can safely assume that these enum values are just going to get hardcoded into the compiled instructions, so there shouldn't be any difference between > > movl eax, 0x12 > > and > > movl eax, 0x23 I'm assuming the histogram code allocates an int[] of size IDBLevelDBBackingStoreInternalErrorMax and then increments each entry as we report it, so the more error codes we have the larger that array has to be. But your argument might assume this and be more advanced, not sure. Is it?
(In reply to comment #15) > I'm assuming the histogram code allocates an int[] of size IDBLevelDBBackingStoreInternalErrorMax and then increments each entry as we report it, so the more error codes we have the larger that array has to be. Yes, that's the extra memory I'm referring to. I think we also keep a copy of the histogram in the browser process and each renderer process that adds to this histogram.