Issue 11572032: Clean up CodeObservers

Issue 11572032: Clean up CodeObservers (Closed)

Created:
8 years ago by Max Heinritz (Google)

Modified:
8 years ago

Reviewers:
siva

CC:
reviews_dartlang.org, cshapiro

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

Visibility:
Public.

More Reviews

Description

Clean up CodeObservers Move CodeObserver registration to the OS abstraction Move Pprof logic to the OS abstraction Add DISALLOW and AllStatic to Code Observers where appropriate BUG=7321 Committed: https://code.google.com/p/dart/source/detail?r=16273 Committed: https://code.google.com/p/dart/source/detail?r=16445

Patch Set 1 : clean up for review #

Total comments: 10

Patch Set 2 : Add code observers for android, other cleanup per Siva's review #

Patch Set 3 : Move pprof to OS abstraction. #

Patch Set 4 : Delete DeleteCodeObservers() from os.h #

Total comments: 1

Patch Set 5 : Add clean up TODO in isolate.cc #

Created: 8 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+389 lines, -311 lines)			Patch
M	runtime/bin/main.cc	View	1 2 3 4	6 chunks	+0 lines, -35 lines	0 comments	Download
M	runtime/bin/run_vm_tests.cc	View	1 2	1 chunk	+4 lines, -15 lines	0 comments	Download
M	runtime/vm/code_observers.h	View	1	4 chunks	+9 lines, -1 line	0 comments	Download
M	runtime/vm/code_observers.cc	View		2 chunks	+6 lines, -121 lines	0 comments	Download
M	runtime/vm/dart_api_impl.cc	View	1 2 3 4	1 chunk	+0 lines, -32 lines	0 comments	Download
D	runtime/vm/debuginfo_macos.cc	View		1 chunk	+0 lines, -50 lines	0 comments	Download
D	runtime/vm/debuginfo_win.cc	View		1 chunk	+0 lines, -50 lines	0 comments	Download
M	runtime/vm/isolate.cc	View	1 2 3 4	3 chunks	+4 lines, -5 lines	0 comments	Download
M	runtime/vm/os.h	View	1 3	1 chunk	+3 lines, -0 lines	0 comments	Download
M	runtime/vm/os_android.cc	View	1 2	2 chunks	+176 lines, -0 lines	0 comments	Download
M	runtime/vm/os_linux.cc	View	1 2	2 chunks	+176 lines, -0 lines	0 comments	Download
M	runtime/vm/os_macos.cc	View	1	1 chunk	+4 lines, -0 lines	0 comments	Download
M	runtime/vm/os_win.cc	View	2	1 chunk	+7 lines, -0 lines	0 comments	Download
M	runtime/vm/vm_sources.gypi	View	1 2	1 chunk	+0 lines, -2 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

siva

os_android.cc also needs some setup done in terms of CodeObservers. https://codereview.chromium.org/11572032/diff/1011/runtime/vm/code_observers.h File runtime/vm/code_observers.h (right): https://codereview.chromium.org/11572032/diff/1011/runtime/vm/code_observers.h#newcode16 ...

8 years ago (2012-12-14 19:13:39 UTC) #2

Max Heinritz (Google)

https://codereview.chromium.org/11572032/diff/1011/runtime/vm/code_observers.h File runtime/vm/code_observers.h (right): https://codereview.chromium.org/11572032/diff/1011/runtime/vm/code_observers.h#newcode16 runtime/vm/code_observers.h:16: public: On 2012/12/14 19:13:40, siva wrote: > CodeObserver() { ...

8 years ago (2012-12-14 19:59:47 UTC) #3

Max Heinritz (Google)

Move pprof logic to OS abstraction to prevent build errors on non-Linux systems.

8 years ago (2012-12-19 22:57:58 UTC) #5

siva

8 years ago (2012-12-20 22:40:15 UTC) #6

lgtm

https://chromiumcodereview.appspot.com/11572032/diff/6018/runtime/vm/isolate.cc
File runtime/vm/isolate.cc (right):

https://chromiumcodereview.appspot.com/11572032/diff/6018/runtime/vm/isolate....
runtime/vm/isolate.cc:530: CodeObservers::DeleteAll();
Can you add a TODO here that this piece of code needs to move
to Dart::Cleanup when we have that method as the cleanup
for Dart::InitOnce

Expand Messages | Collapse Messages