Bug 21071 – JSCore SPI addition for reporting an object's non-GC allocated memory

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Geoffrey Garen]

Patch coming.

Maybe we should make this API some day?

[Geoffrey Garen]

Created attachment 23760 [details]
patch

[Darin Adler]

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

[Geoffrey Garen]

> +        * 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.