RFR: 8292995: improve the SA page cache [v2]
Chris Plummer
cjplummer at openjdk.org
Tue Aug 30 23:02:02 UTC 2022
> The page caching support in SA is woefully dated. I think it has stayed the same for over 20 years when it was originally done for solarix-x86. This code has been replicated for every port. Currently all ports only have a 16mb cache. They use 4k pages and there are 4k of them.
>
> I think the 4k page size is fine. The following comment appears in all the ports:
>
> // ... This is a cache of 4096 4K pages, or 16 MB. The page
> // size must be adjusted to be the hardware's page size.
> // (FIXME: should pick this up from the debugger.)
>
> I disagree with this. Matching the possibly very large hardware page size (I think maybe they meant OS page size) would require the SA page cache to be very very large, using a lot of java heap space. It would also require a lot of unnecessary copying from the debuggee process's memory. There's no reason for the SA cache's page size to match the OS page size.
>
> However, 16mb seems very small. I tried 256mb and this gave about a 10% performance improvement in a heap dump, and is still fairly small, so I think it is a reasonable adjustment.
>
> Another comment you see in all the ports (copied from solaris-x86) is:
>
> // FIXME: re-test necessity of cache on Linux, where data
> // fetching is faster
> // Cache portion of the remote process's address space.
> // Fetching data over the socket connection to dbx is slow.
> // Might be faster if we were using a binary protocol to talk to
> // dbx, but would have to test. For now, this cache works best
> // if it covers the entire heap of the remote process. FIXME: at
>
> At least on linux the cache is definitely needed. I turned it off and a heap dump took 9x longer. Also I think covering the entire heap is overkill, and I doubt was ever being done given how small the cache is. So I think this comment can just be removed.
Chris Plummer has updated the pull request incrementally with one additional commit since the last revision:
Minor indent fix.
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/10069/files
- new: https://git.openjdk.org/jdk/pull/10069/files/9f528186..5b34d066
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=10069&range=01
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=10069&range=00-01
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/10069.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/10069/head:pull/10069
PR: https://git.openjdk.org/jdk/pull/10069
More information about the serviceability-dev
mailing list