<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto"><div dir="ltr"></div><div dir="ltr">Hi,</div><div dir="ltr"><br></div><div dir="ltr">Didn’t read the full conversation, only the part where it was claimed we have byte atomicity on all platforms with memset.</div><div dir="ltr">That’s not true. On SPARC, the default is to use block initializing stores (BIS), which exposes concurrent readers to out-of-thin-air values that were never in the original memory state or the new memory state.</div><div dir="ltr"><br></div><div dir="ltr">Moreover, the compiler is allowed to, and will also in practice, transform a loop that performs memset, into a memset call as long as the memory being memset is non-volatile.</div><div dir="ltr"><br></div><div dir="ltr">These problems were observed in card tables and led to the introduction of </div><h1 id="summary-val" style="margin: 0px 0px 0px -5px; padding: 2px 30px 2px 5px; font-weight: 500; line-height: 1.25; z-index: 1;"><font size="3"><span style="background-color: rgba(255, 255, 255, 0);">memset_with_concurrent_readers() to force the memset not to use BIS.</span></font></h1><div><font size="3"><span style="background-color: rgba(255, 255, 255, 0);"><br></span></font></div><div><font size="3"><span style="background-color: rgba(255, 255, 255, 0);">Just a FYI.</span></font></div><div dir="ltr"><br></div><div dir="ltr">/Erik</div><div dir="ltr"><br>On 15 May 2019, at 07:53, Schmidt, Lutz <<a href="mailto:lutz.schmidt@sap.com">lutz.schmidt@sap.com</a>> wrote:<br><br></div><blockquote type="cite"><div dir="ltr"><span>Hi Vladimir, </span><br><span></span><br><span>thank you for your comments. About filling CodeHeap with bad values after split_block:</span><br><span> - in deallocate_tail, the leading part must remain intact. It contains valid code.</span><br><span> - in search_freelist, one free block is split into two. There I could invalidate the contents of both parts.</span><br><span> - If you want added safety, wouldn't it then be better to invalidate the block contents during add_to_freelist()? You could then be sure there is no executable code in a free block.</span><br><span></span><br><span>Regards,</span><br><span>Lutz</span><br><span></span><br><span>On 14.05.19, 23:00, "Vladimir Kozlov" <<a href="mailto:vladimir.kozlov@oracle.com">vladimir.kozlov@oracle.com</a>> wrote:</span><br><span></span><br><span>    On 5/14/19 1:09 PM, Schmidt, Lutz wrote:</span><br><blockquote type="cite"><span>Hi Vladimir,</span><br></blockquote><blockquote type="cite"><span></span><br></blockquote><blockquote type="cite"><span>I had the same thought re atomicity. memset() is not consistent even on one platform. But I believe it's not a factor here. The original code was a byte-by-byte loop. And we have byte atomicity on all supported platforms, even with memset().</span><br></blockquote><blockquote type="cite"><span></span><br></blockquote><blockquote type="cite"><span>It's a different thing with sequence of initialization. Do we really depend on byte(i) being initialized before byte(i+1)? If so, we would have a problem even with the explicit byte loop. Not on x86, but on ppc with its weak memory ordering.</span><br></blockquote><span></span><br><span>    Okay, if it is byte copy I am fine with it.</span><br><span></span><br><blockquote type="cite"><span></span><br></blockquote><blockquote type="cite"><span>About segment map marking:</span><br></blockquote><blockquote type="cite"><span>There is a short description how the segment map works in heap.cpp, right before CodeHeap::find_start().</span><br></blockquote><blockquote type="cite"><span>In short: each segment map element contains an (unsigned) index which, when subtracted from that element index, addresses the segment map element where the heap block starts. Thus, when you re-initialize the tail part of a heap block range to describe a newly formed heap block, the leading part remains valid.</span><br></blockquote><blockquote type="cite"><span></span><br></blockquote><blockquote type="cite"><span>Segmap  before             after</span><br></blockquote><blockquote type="cite"><span>Index    split             split</span><br></blockquote><blockquote type="cite"><span>   I       0 <- block start   0 <- block start (now shorter)</span><br></blockquote><blockquote type="cite"><span>   I+1     1                  1    each index 0..9 still points</span><br></blockquote><blockquote type="cite"><span>   I+2     2                  2    back to the block start</span><br></blockquote><blockquote type="cite"><span>   I+3     3                  3</span><br></blockquote><blockquote type="cite"><span>   I+4     4                  4</span><br></blockquote><blockquote type="cite"><span>   I+5     5                  5</span><br></blockquote><blockquote type="cite"><span>   I+6     6                  6</span><br></blockquote><blockquote type="cite"><span>   I+7     7                  7</span><br></blockquote><blockquote type="cite"><span>   I+8     8                  8</span><br></blockquote><blockquote type="cite"><span>   I+9     9                  9</span><br></blockquote><blockquote type="cite"><span>   I+10    10                 0 <- new block start</span><br></blockquote><blockquote type="cite"><span>   I+11    11                 1</span><br></blockquote><blockquote type="cite"><span>   I+12    12                 2</span><br></blockquote><blockquote type="cite"><span>   I+13    13                 3</span><br></blockquote><blockquote type="cite"><span>   I+14    14                 4</span><br></blockquote><blockquote type="cite"><span>   I+15    0 <- block start   0 <- block start</span><br></blockquote><blockquote type="cite"><span>   I+16    1                  1</span><br></blockquote><blockquote type="cite"><span>   I+17    2                  2</span><br></blockquote><blockquote type="cite"><span>   I+18    3                  3</span><br></blockquote><blockquote type="cite"><span>   I+19    4                  4</span><br></blockquote><blockquote type="cite"><span></span><br></blockquote><blockquote type="cite"><span>There is a (very short) description about what's happening at the very end of search_freelist(). split_block() is called there as well. Would you like to see a similar comment in deallocate_tail()?</span><br></blockquote><span></span><br><span>    Thank you, I forgot about that first block mapping is still valid.</span><br><span></span><br><span>    What about storing bad value (in debug mode) only in second part and not both parts?</span><br><span></span><br><blockquote type="cite"><span></span><br></blockquote><blockquote type="cite"><span>Once I have your response, I will create a new webrev reflecting your input. I need to do that anyway because the assert in heap.cpp:200 has to go away. It fires spuriously. The checks can't be done at that place. In addition, I will add one line of comment and rename a local variable. That's it.</span><br></blockquote><span></span><br><span>    Okay.</span><br><span></span><br><span>    Thanks,</span><br><span>    Vladimir</span><br><span></span><br><blockquote type="cite"><span></span><br></blockquote><blockquote type="cite"><span>Thanks,</span><br></blockquote><blockquote type="cite"><span>Lutz</span><br></blockquote><blockquote type="cite"><span></span><br></blockquote><blockquote type="cite"><span></span><br></blockquote><blockquote type="cite"><span>On 14.05.19, 20:53, "hotspot-compiler-dev on behalf of Vladimir Kozlov" <<a href="mailto:hotspot-compiler-dev-bounces@openjdk.java.net">hotspot-compiler-dev-bounces@openjdk.java.net</a> on behalf of <a href="mailto:vladimir.kozlov@oracle.com">vladimir.kozlov@oracle.com</a>> wrote:</span><br></blockquote><blockquote type="cite"><span></span><br></blockquote><blockquote type="cite"><span>     Good.</span><br></blockquote><blockquote type="cite"><span></span><br></blockquote><blockquote type="cite"><span>     Do we need to be concern about atomicity of marking? We know that memset() is not atomic (may be I am wrong here).</span><br></blockquote><blockquote type="cite"><span>     An other thing is I did not get logic in deallocate_tail(). split_block() marks only second half of split segments as</span><br></blockquote><blockquote type="cite"><span>     used and (after call) store bad values in it. What about first part? May be add comment.</span><br></blockquote><blockquote type="cite"><span></span><br></blockquote><blockquote type="cite"><span>     Thanks,</span><br></blockquote><blockquote type="cite"><span>     Vladimir</span><br></blockquote><blockquote type="cite"><span></span><br></blockquote><blockquote type="cite"><span>     On 5/14/19 3:47 AM, Schmidt, Lutz wrote:</span><br></blockquote><blockquote type="cite"><blockquote type="cite"><span>Dear all,</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span></span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>May I please request reviews for my change?</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>Bug:    <a href="https://bugs.openjdk.java.net/browse/JDK-8223444">https://bugs.openjdk.java.net/browse/JDK-8223444</a></span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>Webrev: <a href="https://cr.openjdk.java.net/~lucy/webrevs/8223444.00/">https://cr.openjdk.java.net/~lucy/webrevs/8223444.00/</a></span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span></span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>What this change is all about:</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>------------------------------</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>While working on another topic, I came across the code in share/memory/heap.cpp. I applied some small changes which I would call improvements.</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span></span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>Furthermore, and in particular with these changes, the platform-specific parameter CodeCacheMinBlockLength should by fine-tuned to minimize the number of residual small free blocks. Heap block allocation does not create free blocks smaller than CodeCacheMinBlockLength. This parameter value should match the minimal requested heap block size. If it is too small, such free blocks will never be re-allocated. The only chance for them to vanish is when a block next to them gets freed. Otherwise, they linger around (mostly at the beginning of) the free list, slowing down the free block search.</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span></span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>The following free block counts have been found after running JVM98 with different CodeCacheMinBlockLength values. I have used -XX:+PrintCodeHeapAnalytics to see the CodeHeap state at VM shutdown.</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span></span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>JDK-8223444 not applied</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>=======================</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span></span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>         Segment  |  free blocks with CodeCacheMinBlockLength=</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>           Size   |       1      2      3      4      6      8</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>-----------------+-------------------------------------------</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>aarch      128   |       0    153     75     30     38      2</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>ppc        128   |       0    149     98     59     14      2</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>ppcle      128   |       0    219    161    110     69     34</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>s390       256   |       0    142     93     59     30     10</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>x86        128   |       0    215    157    118     42     11</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span></span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span></span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>JDK-8223444 applied</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>===================</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span></span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>         Segment  |  free blocks with CodeCacheMinBlockLength=  |  suggested</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>           Size   |       1      2      3      4      6      8  |   setting</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>-----------------+---------------------------------------------+------------</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>aarch      128   |     221    115     80     36      7      1  |     6</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>ppc        128   |     245    152    101     54     14      4  |     6</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>ppcle      128   |     243    144     89     72     20      5  |     6</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>s390       256   |     168     60     67      8      6      2  |     4</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>x86        128   |     223    139     83     50     11      2  |     6</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span></span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>Thank you for your time and opinion!</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span>Lutz</span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span></span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span></span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span></span><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><span></span><br></blockquote></blockquote><blockquote type="cite"><span></span><br></blockquote><blockquote type="cite"><span></span><br></blockquote><span></span><br><span></span><br></div></blockquote></body></html>