Issue 10538022: Do not reuse tags_ field to store next_ pointer of FreeListElement.

Issue 10538022: Do not reuse tags_ field to store next_ pointer of FreeListElement. (Closed)

Created:
8 years, 6 months ago by Vyacheslav Egorov (Google)

Modified:
8 years, 6 months ago

Reviewers:
Ivan Posva

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Do not reuse tags_ field to store next_ pointer of FreeListElement. Make tags_ field be valid (i.e. encode size and class id) for free list elements. R=iposva@google.com BUG= TEST= Committed: https://code.google.com/p/dart/source/detail?r=8437

Patch Set 1 #

Total comments: 2

Patch Set 2 : Allocate a real class for a free list element. #

Total comments: 4

Created: 8 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+61 lines, -42 lines)			Patch
M	runtime/vm/class_table.cc	View	1	2 chunks	+2 lines, -0 lines	0 comments	Download
M	runtime/vm/freelist.h	View	1	1 chunk	+28 lines, -17 lines	2 comments	Download
M	runtime/vm/freelist.cc	View	1	3 chunks	+14 lines, -4 lines	0 comments	Download
M	runtime/vm/object.cc	View	1	1 chunk	+4 lines, -0 lines	0 comments	Download
M	runtime/vm/raw_object.h	View		4 chunks	+4 lines, -6 lines	2 comments	Download
M	runtime/vm/raw_object.cc	View	1	5 chunks	+9 lines, -15 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

Ivan Posva

https://chromiumcodereview.appspot.com/10538022/diff/1/runtime/vm/class_table.cc File runtime/vm/class_table.cc (right): https://chromiumcodereview.appspot.com/10538022/diff/1/runtime/vm/class_table.cc#newcode76 runtime/vm/class_table.cc:76: // residing outside of the heap. Do not visit ...

8 years, 6 months ago (2012-06-08 07:19:20 UTC) #2

Ivan Posva

LGTM with comments. -Ivan https://chromiumcodereview.appspot.com/10538022/diff/5001/runtime/vm/freelist.h File runtime/vm/freelist.h (right): https://chromiumcodereview.appspot.com/10538022/diff/5001/runtime/vm/freelist.h#newcode45 runtime/vm/freelist.h:45: static bool IsInstance() { return ...

8 years, 6 months ago (2012-06-08 12:50:20 UTC) #4

Vyacheslav Egorov (Google)

8 years, 6 months ago (2012-06-08 12:58:27 UTC) #5

Thanks for the review. Landing.

https://chromiumcodereview.appspot.com/10538022/diff/5001/runtime/vm/freelist.h
File runtime/vm/freelist.h (right):

https://chromiumcodereview.appspot.com/10538022/diff/5001/runtime/vm/freelist...
runtime/vm/freelist.h:45: static bool IsInstance() { return true; }
On 2012/06/08 12:50:20, Ivan Posva wrote:
> private:
>   DISALLOW_ALLOCATION();
>   DISALLOW_IMPLICIT_CONSTRUCTORS(FakeInstance);

Did
 private:
  DISALLOW_ALLOCATION();
  DISALLOW_COPY_AND_ASSIGN(FakeInstance);

Default constructor is required by Class::New<FakeInstance>.

https://chromiumcodereview.appspot.com/10538022/diff/5001/runtime/vm/raw_obje...
File runtime/vm/raw_object.h (right):

https://chromiumcodereview.appspot.com/10538022/diff/5001/runtime/vm/raw_obje...
runtime/vm/raw_object.h:192: class FreeBit : public BitField<bool, kFreeBit, 1>
{};
On 2012/06/08 12:50:20, Ivan Posva wrote:
> Do you still need to make this public?

We still initialize it in the FreeListElement::AsElement.

Moved to private, made FreeListElement a friend of RawClass.

Expand Messages | Collapse Messages