Bug 200541 - OpenSource MemoryFootprint API for JSC command line tool
Summary: OpenSource MemoryFootprint API for JSC command line tool
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-08-08 10:36 PDT by Michael Saboff
Modified: 2019-08-08 13:49 PDT (History)
10 users (show)

See Also:


Attachments
Patch (9.77 KB, patch)
2019-08-08 10:55 PDT, Michael Saboff
saam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2019-08-08 10:36:24 PDT
The APIs needed for the memory peak are available in the current public SDKs for macOS and iOS, therefore these implementations can move to OpenSource.
Comment 1 Michael Saboff 2019-08-08 10:55:09 PDT
Created attachment 375821 [details]
Patch
Comment 2 EWS Watchlist 2019-08-08 10:57:47 PDT
Attachment 375821 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/spi/darwin/ProcessMemoryFootprint.h:35:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WTF/wtf/spi/darwin/ProcessMemoryFootprint.h:35:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Source/WTF/wtf/spi/darwin/ProcessMemoryFootprint.h:36:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WTF/wtf/spi/darwin/ProcessMemoryFootprint.h:36:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Source/WTF/wtf/spi/darwin/ProcessMemoryFootprint.h:38:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 5 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Saam Barati 2019-08-08 11:51:35 PDT
Comment on attachment 375821 [details]
Patch

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

r=me

> Source/JavaScriptCore/jsc.cpp:144
> +    MemoryFootprint(const ProcessMemoryFootprint& o)
> +        : ProcessMemoryFootprint(o)

style nit: let's give it a name instead of "o"

> Source/WTF/wtf/spi/darwin/ProcessMemoryFootprint.h:46
> +#if !PLATFORM(IOS_FAMILY_SIMULATOR) && __has_include(<libproc.h>)
> +#    include <libproc.h>
> +#    if RUSAGE_INFO_CURRENT >= 4
> +#        define HAS_MAX_FOOTPRINT
> +#        if defined(RLIMIT_FOOTPRINT_INTERVAL) && __has_include(<libproc_internal.h>) \
> +             && ((PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 120000) \
> +                 || (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400))
> +#            define HAS_RESET_FOOTPRINT_INTERVAL
> +#            define MAX_FOOTPRINT_FIELD ri_interval_max_phys_footprint
> +#            include <libproc_internal.h>
> +#        else
> +#            define MAX_FOOTPRINT_FIELD ri_lifetime_max_phys_footprint
> +#        endif
> +#    else
> +#        define HAS_ONLY_PHYS_FOOTPRINT
> +#    endif
> +#endif

I respect the indentation here
Comment 4 Michael Saboff 2019-08-08 13:38:29 PDT
(In reply to Saam Barati from comment #3)
> Comment on attachment 375821 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=375821&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/jsc.cpp:144
> > +    MemoryFootprint(const ProcessMemoryFootprint& o)
> > +        : ProcessMemoryFootprint(o)
> 
> style nit: let's give it a name instead of "o"

Called it "src".
Comment 5 Michael Saboff 2019-08-08 13:48:39 PDT
Committed r248441: <https://trac.webkit.org/changeset/248441>
Comment 6 Radar WebKit Bug Importer 2019-08-08 13:49:17 PDT
<rdar://problem/54094516>