Ticket #1605 (closed patch: fixed)

Opened 5 years ago

Last modified 4 years ago

Documentation for buffers in include/parrot/pobj.h is outdated.

Reported by: bacek Owned by: Paul C. Anagnostopoulos
Priority: major Milestone: 2.6
Component: core Version: trunk
Severity: medium Keywords:
Cc: Language:
Patch status: new Platform:

Description

Hello.

After merging compact_pool_revamp branch documentation in pobj.h and gc/alloc_resources.c doesn't reflect reality anymore and require major update.

Basically storage header consists of 3 fields:

  • Pointer to Memory_Block* from which it was allocated.
  • Buffer_moved_FLAG - set to 1 during compacting when we move shared buffer
  • Buffer_shared_FLAG - set to 1 when we share storage between different STRINGs. (Replacement for old PObj_COW_FLAG)

My Englsih-fu is weak for this task but I'll be able to answer questions on IRC or ticket.

Attachments

PCA-patch-001.patch Download (22.7 KB) - added by Paul C. Anagnostopoulos 4 years ago.

Change History

Changed 4 years ago by Paul C. Anagnostopoulos

I was going to attack this issue by updating the Memory Internals developer document, but it is sadly out-of-date. So I took a step back and started commenting all the relevant data structures in gc_private.h. That way I can learn how everything hangs together and ultimately revise the developer document.

Changed 4 years ago by Paul C. Anagnostopoulos

  • owner set to Paul C. Anagnostopoulos
  • status changed from new to assigned

Changed 4 years ago by Paul C. Anagnostopoulos

  • type changed from cage to patch

I have edited gc_private.h to document all the memory management data structures. I also added my name to the CREDITS file.

My next step is to correct the comments in pobj.h and alloc_resources.c to reflect the current layout of buffers.

Changed 4 years ago by Paul C. Anagnostopoulos

I worked on the comments in pobj.h and alloc_resources.c. They are now much enhanced and up-to-date. In particular, Buffers are described correctly.

I also renamed a couple of the Buffer_xxx macros so that they accurately reflect reality.

My next step is to rework the Memory Internals developer document.

Changed 4 years ago by Paul C. Anagnostopoulos

  • patch set to new

Changed 4 years ago by Paul C. Anagnostopoulos

Changed 4 years ago by Paul C. Anagnostopoulos

My use of the terms descriptor and header in pobj.h was inconsistent with the rest of the system. I reworked the comments to use header and prolog instead.

Changed 4 years ago by Paul C. Anagnostopoulos

  • component changed from docs to core

Changed 4 years ago by Paul C. Anagnostopoulos

Note to committers: You can commit PCA-patch-001 without waiting for the second patch. The second one will update the Memory Internals document.

Changed 4 years ago by cotto

Do you have an updated patch? The recent gc_threshold_tuning branch seems to have caused many conflicts with the patch.

Changed 4 years ago by Paul C. Anagnostopoulos

I was afraid this was going to happen. Since I'm leaving for the beach today, I think you'll have to abandon the patch and let me rework all the files after I return. Other files may appear okay, but some depend on changes to pobj.h

You could throw in my change to CREDITS.

Changed 4 years ago by cotto

I fixed the conflicts and applied the patch as r48492. The patch looked ok to me, but it could use some review for accuracy by someone who knows how to navigate the GC code better than I do.

Changed 4 years ago by Paul C. Anagnostopoulos

I really appreciate that, cotto. Especially while I'm lying on the beach. I'm working on a rewrite of Memory Internals developer document in my free time here, in between my other free time.

Changed 4 years ago by cotto

  • status changed from assigned to closed
  • resolution set to fixed

bacek is happy with the accuracy of the docs, so this ticket is now closed. Thanks for contributing!

Note: See TracTickets for help on using tickets.