Bug 89661 - Web Inspector: Properly display native memory sizes bigger than 2GB
Summary: Web Inspector: Properly display native memory sizes bigger than 2GB
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexei Filippov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-21 07:20 PDT by Alexei Filippov
Modified: 2012-06-21 21:08 PDT (History)
11 users (show)

See Also:


Attachments
screenshot (74.39 KB, image/png)
2012-06-21 07:20 PDT, Alexei Filippov
no flags Details
Patch (4.84 KB, patch)
2012-06-21 07:25 PDT, Alexei Filippov
no flags Details | Formatted Diff | Diff
Patch (4.71 KB, patch)
2012-06-21 08:12 PDT, Alexei Filippov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexei Filippov 2012-06-21 07:20:19 PDT
Created attachment 148795 [details]
screenshot

Currently when a component size exceeds 2GB it is shown as a negative value.
Comment 1 Alexei Filippov 2012-06-21 07:25:43 PDT
Created attachment 148796 [details]
Patch
Comment 2 Yury Semikhatsky 2012-06-21 07:32:54 PDT
Comment on attachment 148796 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148796&action=review

> Source/WebCore/inspector/InspectorMemoryAgent.cpp:340
> +    jsHeapAllocated->setSize(static_cast<double>(totalJSHeapSize));

int to double conversion shouldn't require static_cast, pleas remove it.
Comment 3 Alexei Filippov 2012-06-21 07:41:54 PDT
Comment on attachment 148796 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148796&action=review

>> Source/WebCore/inspector/InspectorMemoryAgent.cpp:340
>> +    jsHeapAllocated->setSize(static_cast<double>(totalJSHeapSize));
> 
> int to double conversion shouldn't require static_cast, pleas remove it.

It is size_t which is 64-bits long on x64, thus it may lead to precision loss when converted to double, thus it should require a cast.
Comment 4 Alexander Pavlov (apavlov) 2012-06-21 07:58:35 PDT
Comment on attachment 148796 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148796&action=review

>>> Source/WebCore/inspector/InspectorMemoryAgent.cpp:340
>>> +    jsHeapAllocated->setSize(static_cast<double>(totalJSHeapSize));
>> 
>> int to double conversion shouldn't require static_cast, pleas remove it.
> 
> It is size_t which is 64-bits long on x64, thus it may lead to precision loss when converted to double, thus it should require a cast.

So, what will static_cast do in this case?
Comment 5 Yury Semikhatsky 2012-06-21 08:01:37 PDT
(In reply to comment #3)
> (From update of attachment 148796 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=148796&action=review
> > int to double conversion shouldn't require static_cast, pleas remove it.
> 
> It is size_t which is 64-bits long on x64, thus it may lead to precision loss when converted to double, thus it should require a cast.

But C++ compiler won't complain in that case.
Comment 6 Alexei Filippov 2012-06-21 08:03:23 PDT
(In reply to comment #4)
> (From update of attachment 148796 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=148796&action=review
> 
> >>> Source/WebCore/inspector/InspectorMemoryAgent.cpp:340
> >>> +    jsHeapAllocated->setSize(static_cast<double>(totalJSHeapSize));
> >> 
> >> int to double conversion shouldn't require static_cast, pleas remove it.
> > 
> > It is size_t which is 64-bits long on x64, thus it may lead to precision loss when converted to double, thus it should require a cast.
> 
> So, what will static_cast do in this case?

Convert uint64 to double losing precision.
If I remove the cast it will also do the conversion on some platforms while reporting warnings and errors on other platforms & flag settings.
Comment 7 Alexei Filippov 2012-06-21 08:04:02 PDT
(In reply to comment #5)
> (In reply to comment #3)
> > (From update of attachment 148796 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=148796&action=review
> > > int to double conversion shouldn't require static_cast, pleas remove it.
> > 
> > It is size_t which is 64-bits long on x64, thus it may lead to precision loss when converted to double, thus it should require a cast.
> 
> But C++ compiler won't complain in that case.

Ok, if you insist I'll remove the cast.
Comment 8 Alexei Filippov 2012-06-21 08:12:14 PDT
Created attachment 148806 [details]
Patch
Comment 9 WebKit Review Bot 2012-06-21 21:07:57 PDT
Comment on attachment 148806 [details]
Patch

Clearing flags on attachment: 148806

Committed r121002: <http://trac.webkit.org/changeset/121002>
Comment 10 WebKit Review Bot 2012-06-21 21:08:02 PDT
All reviewed patches have been landed.  Closing bug.