فهرست منبع

Kernel: Make sys$perf_register_string() generate the string ID's

Making userspace provide a global string ID was silly, and made the API
extremely difficult to use correctly in a global profiling context.

Instead, simply make the kernel do the string ID allocation for us.
This also allows us to convert the string storage to a Vector in the
kernel (and an array in the JSON profile data.)
Andreas Kling 3 سال پیش
والد
کامیت
1e90a3a542

+ 8 - 7
Kernel/PerformanceEventBuffer.cpp

@@ -167,9 +167,9 @@ template<typename Serializer>
 bool PerformanceEventBuffer::to_json_impl(Serializer& object) const
 {
     {
-        auto strings = object.add_object("strings");
+        auto strings = object.add_array("strings");
         for (auto& it : m_strings) {
-            strings.add(String::number(it.key), it.value->view());
+            strings.add(it.view());
         }
     }
 
@@ -305,13 +305,14 @@ void PerformanceEventBuffer::add_process(const Process& process, ProcessEventTyp
     }
 }
 
-KResult PerformanceEventBuffer::register_string(FlatPtr string_id, NonnullOwnPtr<KString> string)
+KResultOr<FlatPtr> PerformanceEventBuffer::register_string(NonnullOwnPtr<KString> string)
 {
-    m_strings.set(string_id, move(string));
+    FlatPtr string_id = m_strings.size();
 
-    // FIXME: Switch m_strings to something that can signal allocation failure,
-    //        and then propagate such failures here.
-    return KSuccess;
+    if (!m_strings.try_append(move(string)))
+        return ENOBUFS;
+
+    return string_id;
 }
 
 }

+ 2 - 2
Kernel/PerformanceEventBuffer.h

@@ -120,7 +120,7 @@ public:
 
     void add_process(const Process&, ProcessEventType event_type);
 
-    KResult register_string(FlatPtr string_id, NonnullOwnPtr<KString>);
+    KResultOr<FlatPtr> register_string(NonnullOwnPtr<KString>);
 
 private:
     explicit PerformanceEventBuffer(NonnullOwnPtr<KBuffer>);
@@ -133,7 +133,7 @@ private:
     size_t m_count { 0 };
     NonnullOwnPtr<KBuffer> m_buffer;
 
-    HashMap<FlatPtr, NonnullOwnPtr<KString>> m_strings;
+    NonnullOwnPtrVector<KString> m_strings;
 };
 
 extern bool g_profiling_all_threads;

+ 1 - 1
Kernel/Process.h

@@ -400,7 +400,7 @@ public:
     KResultOr<FlatPtr> sys$pledge(Userspace<const Syscall::SC_pledge_params*>);
     KResultOr<FlatPtr> sys$unveil(Userspace<const Syscall::SC_unveil_params*>);
     KResultOr<FlatPtr> sys$perf_event(int type, FlatPtr arg1, FlatPtr arg2);
-    KResultOr<FlatPtr> sys$perf_register_string(FlatPtr string_id, Userspace<char const*>, size_t);
+    KResultOr<FlatPtr> sys$perf_register_string(Userspace<char const*>, size_t);
     KResultOr<FlatPtr> sys$get_stack_bounds(Userspace<FlatPtr*> stack_base, Userspace<size_t*> stack_size);
     KResultOr<FlatPtr> sys$ptrace(Userspace<const Syscall::SC_ptrace_params*>);
     KResultOr<FlatPtr> sys$sendfd(int sockfd, int fd);

+ 2 - 2
Kernel/Syscalls/perf_event.cpp

@@ -21,7 +21,7 @@ KResultOr<FlatPtr> Process::sys$perf_event(int type, FlatPtr arg1, FlatPtr arg2)
     return events_buffer->append(type, arg1, arg2, nullptr);
 }
 
-KResultOr<FlatPtr> Process::sys$perf_register_string(FlatPtr string_id, Userspace<char const*> user_string, size_t user_string_length)
+KResultOr<FlatPtr> Process::sys$perf_register_string(Userspace<char const*> user_string, size_t user_string_length)
 {
     VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this)
     auto* events_buffer = current_perf_events_buffer();
@@ -32,7 +32,7 @@ KResultOr<FlatPtr> Process::sys$perf_register_string(FlatPtr string_id, Userspac
     if (string_or_error.is_error())
         return string_or_error.error();
 
-    return events_buffer->register_string(string_id, string_or_error.release_value());
+    return events_buffer->register_string(string_or_error.release_value());
 }
 
 }

+ 6 - 7
Userland/DevTools/Profiler/Profile.cpp

@@ -209,15 +209,14 @@ Result<NonnullOwnPtr<Profile>, String> Profile::load_from_perfcore_file(const St
         kernel_elf = make<ELF::Image>(file_or_error.value()->bytes());
 
     auto strings_value = object.get_ptr("strings"sv);
-    if (!strings_value || !strings_value->is_object())
-        return String { "Malformed profile (strings is not an object)" };
+    if (!strings_value || !strings_value->is_array())
+        return String { "Malformed profile (strings is not an array)" };
 
     HashMap<FlatPtr, String> profile_strings;
-    strings_value->as_object().for_each_member([&](String const& key, JsonValue const& value) {
-        auto string_id = key.to_uint();
-        VERIFY(string_id.has_value());
-        profile_strings.set(string_id.value(), value.to_string());
-    });
+    for (FlatPtr string_id = 0; string_id < strings_value->as_array().size(); ++string_id) {
+        auto& value = strings_value->as_array().at(string_id);
+        profile_strings.set(string_id, value.to_string());
+    }
 
     auto events_value = object.get_ptr("events");
     if (!events_value || !events_value->is_array())

+ 2 - 2
Userland/Libraries/LibC/serenity.cpp

@@ -95,9 +95,9 @@ int perf_event(int type, uintptr_t arg1, FlatPtr arg2)
     __RETURN_WITH_ERRNO(rc, rc, -1);
 }
 
-int perf_register_string(uintptr_t string_id, char const* string, size_t string_length)
+int perf_register_string(char const* string, size_t string_length)
 {
-    int rc = syscall(SC_perf_register_string, string_id, string, string_length);
+    int rc = syscall(SC_perf_register_string, string, string_length);
     __RETURN_WITH_ERRNO(rc, rc, -1);
 }
 

+ 1 - 1
Userland/Libraries/LibC/serenity.h

@@ -119,7 +119,7 @@ enum {
 #define PERF_EVENT_MASK_ALL (~0ull)
 
 int perf_event(int type, uintptr_t arg1, uintptr_t arg2);
-int perf_register_string(uintptr_t string_id, char const* string, size_t string_length);
+int perf_register_string(char const* string, size_t string_length);
 
 int get_stack_bounds(uintptr_t* user_stack_base, size_t* user_stack_size);
 

+ 2 - 2
Userland/Libraries/LibJS/Heap/Heap.cpp

@@ -26,7 +26,7 @@
 namespace JS {
 
 #ifdef __serenity__
-static constexpr FlatPtr gc_perf_string_id = 0x13378086;
+static int gc_perf_string_id;
 #endif
 
 Heap::Heap(VM& vm)
@@ -34,7 +34,7 @@ Heap::Heap(VM& vm)
 {
 #ifdef __serenity__
     auto gc_signpost_string = "Garbage collection"sv;
-    perf_register_string(gc_perf_string_id, gc_signpost_string.characters_without_null_termination(), gc_signpost_string.length());
+    gc_perf_string_id = perf_register_string(gc_signpost_string.characters_without_null_termination(), gc_signpost_string.length());
 #endif
 
     if constexpr (HeapBlock::min_possible_cell_size <= 16) {