[Webkit-unassigned] [Bug 67993] Prediction tracking is not precise enough

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=67993





--- Comment #5 from Geoffrey Garen <ggaren at apple.com>  2011-09-13 17:35:44 PST ---
(From update of attachment 107236)
View in context: https://bugs.webkit.org/attachment.cgi?id=107236&action=review

> Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:813
> +		0FD82E82141F3FC900179C94 /* BoundsCheckedPointer.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = BoundsCheckedPointer.h; sourceTree = "<group>"; };
> +		0FD82E84141F3FDA00179C94 /* PredictedType.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PredictedType.cpp; sourceTree = "<group>"; };

You'll need to add these guys to the other project files (JavaScriptCore.vcproj, etc.).

> Source/JavaScriptCore/bytecode/PredictedType.h:42
> +static const PredictedType PredictObjectOther   = 0x0010; // It's definitely an object but not JSFinalObject or JSArray.
> +static const PredictedType PredictSomeObject    = 0x0020; // It's some unknown subclass of JSObject.
> +static const PredictedType PredictObject        = 0x003f; // It's definitely a subclass of JSObject.

The distinction between these three object prediction terms is super confusing to me. Can you clarify somehow?

> Source/JavaScriptCore/bytecode/PredictedType.h:60
>  enum PredictionSource { WeakPrediction, StrongPrediction };
>  
>  inline bool isCellPrediction(PredictedType value)
>  {
> -    return (value & PredictCell) == PredictCell && !(value & ~(PredictArray | PredictionTagMask));
> +    return !!(value & PredictCell) && !(value & ~(PredictCell | PredictionTagMask));
> +}

Seems like this could all become an object with member functions, wrapping a uint16_t. Not necessary in this patch, though.

> Source/JavaScriptCore/wtf/BoundsCheckedPointer.h:7
> + *  This library is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU Library General Public
> + *  License as published by the Free Software Foundation; either
> + *  version 2 of the License, or (at your option) any later version.

Is this newly authored code or LGPL2 code from somewhere else? If newly authored, BSD license, please.

> Source/JavaScriptCore/wtf/BoundsCheckedPointer.h:38
> +class BoundsCheckedPointer {

Probably overkill to do in this patch, but it seems like this class could become an iterator for FixedArray<typename, size_t>. That would simplify the use case a bit, and guarantee that the data and the bounds checked pointer were in agreement about the data's size.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.