Bug 146874 – Watchpoints should be allocated with FastMalloc

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

[Filip Pizlo]

Previously we had some cruft about allocating some kinds of Watchpoints in SegmentedVectors.  That's pretty sad.  Also, it appears that some of the watchpoints are allocated with normal malloc.  Also sad.

[Filip Pizlo]

Created attachment 256645 [details]
the patch

[Filip Pizlo]

Comment on attachment 256645 [details]
the patch

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

> Source/JavaScriptCore/bytecode/CodeBlockJettisoningWatchpoint.h:2
> - * Copyright (C) 2013 Apple Inc. All rights reserved.
> + * Copyright (C) 2013, 2015 Apple Inc. All rights reserved.

Oops, I'll revert.  I just deleted code.

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/186705

[Darin Adler]

Comment on attachment 256645 [details]
the patch

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

> Source/JavaScriptCore/dfg/DFGDesiredWatchpoints.h:95
>          typename HashSet<WatchpointSetType*>::iterator iter = m_sets.begin();
>          typename HashSet<WatchpointSetType*>::iterator end = m_sets.end();
> -        for (; iter != end; ++iter) {
> -            common.watchpoints.append(CodeBlockJettisoningWatchpoint(codeBlock));
> -            Adaptor::add(codeBlock, *iter, &common.watchpoints.last());
> -        }
> +        for (; iter != end; ++iter)
> +            Adaptor::add(codeBlock, *iter, common.watchpoints.add(codeBlock));

Here’s another way to write this:

    for (auto* set : m_sets)
        Adaptor::add(codeBlock, set, common.watchpoints.add(codeBlock));

I think the modern for loop makes this code look clearer.

[Darin Adler]

Comment on attachment 256645 [details]
the patch

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

>> Source/JavaScriptCore/dfg/DFGDesiredWatchpoints.h:95
>> +            Adaptor::add(codeBlock, *iter, common.watchpoints.add(codeBlock));
> 
> Here’s another way to write this:
> 
>     for (auto* set : m_sets)
>         Adaptor::add(codeBlock, set, common.watchpoints.add(codeBlock));
> 
> I think the modern for loop makes this code look clearer.

Just noticed you did a variation on that in your very next patch. Never mind!