Bug 21071 - JSCore SPI addition for reporting an object's non-GC allocated memory
Summary: JSCore SPI addition for reporting an object's non-GC allocated memory
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-09-24 12:33 PDT by Geoffrey Garen
Modified: 2008-09-24 13:39 PDT (History)
0 users

See Also:


Attachments
patch (10.81 KB, patch)
2008-09-24 12:34 PDT, Geoffrey Garen
darin: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2008-09-24 12:33:58 PDT
Patch coming.

Maybe we should make this API some day?
Comment 1 Geoffrey Garen 2008-09-24 12:34:10 PDT
Created attachment 23760 [details]
patch
Comment 2 Darin Adler 2008-09-24 12:41:14 PDT
Comment on attachment 23760 [details]
patch

+        * API/JSBase.h: Filled in some missing function names.

Good, but separate. Probably should land separately (first?).

+        * JavaScriptCore.xcodeproj/project.pbxproj:
+        
+        ^ Add the SPI.

Cute, but unconventional. Should we really use this style? How about just having the comment in a paragraph with the filenames.

Why SPI and not API? Is this just a staging thing? It seems strange and not helpful to make a whole new SPI.cpp file to put this function in if its SPI status will change later.

The normal term for SPI in header files is "Private". The header a new SPI function should go into is whatever header the function would go into if it was API, but with "Private.h" at the end instead of ".h" or "Internal.h".

review- because JSSPI.h is not the right name for this header
Comment 3 Geoffrey Garen 2008-09-24 13:39:07 PDT
> +        * API/JSBase.h: Filled in some missing function names.
> 
> Good, but separate. Probably should land separately (first?).

Fixed.

> +        ^ Add the SPI.
> 
> Cute, but unconventional. Should we really use this style? How about just
> having the comment in a paragraph with the filenames.

I'm not sure what the best style is.

I don't like putting the comment next to the file name in a file that's truncated to 80 characters, because the first 20-75 characters on the line can be taken up just by the file name. It makes reading the actual comment hard.

I also don't think that style works well when you've made one change to many files. Stating the exact same change next to each file name seems silly. Stating the change only on the first or last line seems to imply that the change was only made to the first or last file.

I guess I'll state the change next to each file name. Copy-paste makes this easy. But I think it makes for a hard-to-read ChangeLog.

> Why SPI and not API? Is this just a staging thing? It seems strange and not
> helpful to make a whole new SPI.cpp file to put this function in if its SPI
> status will change later.

It's staging. I'm still unclear on whether the function should be per-context or per-object.

> The normal term for SPI in header files is "Private". The header a new SPI
> function should go into is whatever header the function would go into if it was
> API, but with "Private.h" at the end instead of ".h" or "Internal.h".

Fixed.

Committed revision 36863.