This revision was automatically updated to reflect the committed changes.
Closed by commit rL250814: Increase default memory cache line size for android
(authored by labath).
Changed prior to commit:
http://reviews.llvm.org/D13812?vs=37776=37845#toc
Repository:
rL LLVM
tberghammer added a comment.
Looks good with a few minor comments inline
One additional design question:
What is your opinion about specifying the default value for the memory cache
line size (the 512 byte) in Platform::GetDefaultMemoryCacheLineSize() instead
of in the property definition? I
labath added a comment.
Good catch. fixed and committed.
Repository:
rL LLVM
http://reviews.llvm.org/D13812
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
tberghammer added a comment.
I agree with Greg that the Platform should hand out the value but I would
suggest to create a Platform::GetMemoryCacheLineSize() function what will be
overwritten in PlatfromAndroid.
> ADB packets have a fixed size of 4k.
It is not true. ADB packets have a
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
Looks good. The other way to do this would be to add a key/value pair to the
qHostInfo like "memory_cache_line_byte_size" and return a value that makes
sense for the current Andriod debug
labath updated this revision to Diff 37776.
labath added a comment.
Adress review comments.
http://reviews.llvm.org/D13812
Files:
include/lldb/Target/Platform.h
source/Plugins/Platform/Android/PlatformAndroid.cpp
source/Plugins/Platform/Android/PlatformAndroid.h
labath added a comment.
New version of the patch. I agree that it looks better like this.
I have put the setting logic in the process constructor, rather than doing it
lazily - if anyone was observing the setting value, it would seem strange that
the value of setting changed suddenly after the
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
I would rather you be able to ask the current platform for the default memory
cache line size and have lldb_private::Process do this automatically on the
first memory read if
labath created this revision.
labath added reviewers: clayborg, tberghammer.
labath added a subscriber: lldb-commits.
Herald added subscribers: srhines, danalbert, tberghammer.
ADB packets have a fixed size of 4k. This means the size of memory reads does
not affect speed
too much (as long as it
