Review request for JDK-8136700: Make sure Context.anonymousHostClasses doesn't grow unbounded
Hannes Wallnoefer
hannes.wallnoefer at oracle.com
Fri Sep 18 14:13:52 UTC 2015
Am 2015-09-18 um 15:59 schrieb Sundararajan Athijegannathan:
> I presume it is for code clarity sake? That is a private method in
> that class called only from the other synchronized method. How about
> just adding an assert using Thread.holdsLock?
Well it's mostly as a guard for potential changes down the road. Nothing
says "this method needs to be synchronized" better than just making it
synchronized IMO. :)
Hannes
> -Sundar
>
> On 9/18/2015 7:14 PM, Hannes Wallnoefer wrote:
>> Shouldn't the methods modifying the map also be synchronized, even if
>> we know they are (currently) only called from a synchronized method?
>>
>> Otherwise looks good.
>>
>> Hannes
>>
>> Am 2015-09-18 um 14:05 schrieb Sundararajan Athijegannathan:
>>> +1
>>>
>>> On 9/18/2015 3:24 PM, Attila Szegedi wrote:
>>>>> On Sep 18, 2015, at 6:22 AM, Sundararajan Athijegannathan
>>>>> <sundararajan.athijegannathan at oracle.com> wrote:
>>>>>
>>>>> * Context.compile is synchronized and so there is no need for
>>>>> ConcurrentHashMap + concurrency related comment.
>>>> You’re right, I removed them. I updated webrev in-place.
>>>>
>>>>> * Map.remove(Object, Object) is used which is since jdk 1.8. If we
>>>>> have to backport this change to jdk8u (where jdk7 is used as
>>>>> JAVA_HOME to build/test), we've to take care of this.
>>>> If we backport, this will show up as a compile error, so we’ll
>>>> adjust then. I’d rather run another round of a short review then
>>>> than have a less efficient construct in 9 codebase.
>>>>
>>>> Thanks,
>>>> Attila.
>>>>
>>>>> -Sundar
>>>>>
>>>>> On 9/17/2015 9:07 PM, Attila Szegedi wrote:
>>>>>> Indeed, that’s correct. A new webrev is available for review at
>>>>>> <http://cr.openjdk.java.net/~attila/8136700/webrev.01.jdk9> that
>>>>>> manually handles removal of cleared entries.
>>>>>>
>>>>>> Attila.
>>>>>>
>>>>>>> On Sep 17, 2015, at 4:29 PM, Michael Haupt
>>>>>>> <michael.haupt at oracle.com> wrote:
>>>>>>>
>>>>>>> Hi again,
>>>>>>>
>>>>>>> no sorry; I have to revoke this as I forgot weak sets aren't
>>>>>>> weak in the way they have to be in this case.
>>>>>>>
>>>>>>> Best,
>>>>>>>
>>>>>>> Michael
>>>>>>>
>>>>>>>> Am 17.09.2015 um 16:18 schrieb Michael Haupt
>>>>>>>> <michael.haupt at oracle.com>:
>>>>>>>>
>>>>>>>> Hi Attila,
>>>>>>>>
>>>>>>>> lower-case thumbs up!
>>>>>>>>
>>>>>>>> Best,
>>>>>>>>
>>>>>>>> Michael
>>>>>>>>
>>>>>>>>> Am 17.09.2015 um 16:14 schrieb Attila Szegedi
>>>>>>>>> <attila.szegedi at oracle.com>:
>>>>>>>>>
>>>>>>>>> Please review JDK-8136700 "Make sure
>>>>>>>>> Context.anonymousHostClasses doesn't grow unbounded" at
>>>>>>>>> <http://cr.openjdk.java.net/~attila/8136700/webrev.jdk9> for
>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8136700>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Attila.
>>>>>>> --
>>>>>>>
>>>>>>> <http://www.oracle.com/>
>>>>>>> Dr. Michael Haupt | Principal Member of Technical Staff
>>>>>>> Phone: +49 331 200 7277 | Fax: +49 331 200 7561
>>>>>>> Oracle Java Platform Group | LangTools Team | Nashorn
>>>>>>> Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467
>>>>>>> Potsdam, Germany
>>>>>>> <http://www.oracle.com/commitment> Oracle is committed to
>>>>>>> developing practices and products that help protect the environment
>>>>>>>
>>>
>>
>
More information about the nashorn-dev
mailing list