Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the documentation concerning GASMAN, add CollectGarbage #4168

Merged
merged 2 commits into from
Nov 26, 2020

Conversation

ThomasBreuer
Copy link
Contributor

@ThomasBreuer ThomasBreuer commented Nov 11, 2020

GAP can use several garbage collectors (GASMAN, the Julia garbage collector, and the Boehm garbage collector).
Up to now, the GAP documentation mentions only GASMAN.
Several GAP functions make sense only if GASMAN is running, and several command line options make sense only in this case.

The proposed changes try to adjust the documentation to situations where GASMAN is not used.
(More changes in this direction will be needed, below is a list of open questions.)

Details of the changes:

  • In debug.xml, section "Global Memory Information",
    moved the introductory part to a new subsection "Garbage Collection"
    that can be cross-referenced from places where garbage collection
    is mentioned;
    mention in this subsection that several garbage collectors
    can be used by GAP.

  • In the functions GasmanStatistics, GasmanMessageStatus,
    GasmanLimits, mention that they work only if GASMAN is running.

  • In run.xml, section "Saving and Loading a Workspace",
    mention that workspaces can be saved/loaded only if GASMAN is running.

  • Added a cross-reference from SaveOnExitFile to SaveWorkspace.

  • Changed the description of GAP's command line options:

    • In run.xml, mention that the options -g, -L, -o, -s
      are available only if GASMAN is used.
    • Do the same in system.g, variable GAPInfo.CommandLineOptionData.
      (I had thought about omitting the not supported options from the
      list, but such a filtered list might confuse users who want to
      understand why their options did not work as expected.)
  • Call GASMAN( "message" ) in SetGasmanMessageStatus
    only if GASMAN is running, in order to avoid an error message.

  • In chapter "Weak Pointers", removed the word GASMAN in the introduction
    and added a reference to "Garbage Collection".
    (Weak pointer objects work also with other garbage collectors than
    GASMAN.)

Open questions:

  • The command line options -K and -m are evaluated also when the
    Julia GC is running, but are their values really used?

  • The introduction of the section "Command Line Options" refers to a file
    https://www.gap-system.org/Download/INSTALL which exists but seems to
    be outdated --which URL should be given here?
    (The corresponding file in the GAP distribution is INSTALL.md.)

  • If GASMAN is not running, a file given with the -L command line option
    is currently ignored, because LoadWorkspace is not called when
    GAP_ENABLE_SAVELOAD is not defined.
    Should we better show an error message telling that no workspace
    could be loaded?
    (In earlier versions, there was such a message.)

  • The function GASMAN is undocumented.
    (The Development manual contains a section "Garbage collection in GAP",
    where GASMAN is said to be "the GAP memory manager".
    This seems to be the right place to add documentation for alternative
    garbage collectors.
    On the other hand, this manual is not part of the GAP distribution.)

    Calling GASMAN( "collect" ) and GASMAN( "partial" ) work also
    with the Julia GC; if this is intentional then we should choose
    a better name for the function.
    (GASMAN( "message" ) works only with the GASMAN GC.)

    The function GASMAN is shown in manual examples about weak pointer
    objects. Thus a function for forcing a garbage collection should be
    documented.

  • The section "The GASMAN Interface for Weak Pointer Objects"
    in chapter "Weak Pointers" looks GASMAN specific (not only the title).
    How to reformulate it?

GAP can use several garbage collectors
(GASMAN, the Julia garbage collector, and the Boehm garbage collector).
Up to now, the GAP documentation mentions only GASMAN.
Several GAP functions make sense only if GASMAN is running,
and several command line options make sense only in this case.

The proposed changes try to adjust the documentation to situations
where GASMAN is not used.
(More changes in this direction will be needed,
below is a list of open questions.)

Details of the changes:
----------------------

- In `debug.xml`, section "Global Memory Information",
  moved the introductory part to a new subsection "Garbage Collection"
  that can be cross-referenced from places where garbage collection
  is mentioned;
  mention in this subsection that several garbage collectors
  can be used by GAP.

- In the functions `GasmanStatistics, `GasmanMessageStatus`,
  `GasmanLimits`, mention that they work only if GASMAN is running.

- In `run.xml`, section "Saving and Loading a Workspace",
  mention that workspaces can be saved/loaded only if GASMAN is running.

- Added a cross-reference from `SaveOnExitFile` to `SaveWorkspace`.

- Changed the description of GAP's command line options:
  - In `run.xml`, mention that the options `-g`, `-L`, `-o`, `-s`
    are available only if GASMAN is used.
  - Do the same in `system.g`, variable `GAPInfo.CommandLineOptionData`.
    (I had thought about omitting the not supported options from the
    list, but such a filtered list might confuse users who want to
    understand why their options did not work as expected.)

- Call `GASMAN( "message" )` in `SetGasmanMessageStatus`
  only if GASMAN is running, in order to avoid an error message.

- In chapter "Weak Pointers", removed the word GASMAN in the introduction
  and added a reference to "Garbage Collection".
  (Weak pointer objects work also with other garbage collectors than
  GASMAN.)

Open questions:
--------------

- The command line options `-K` and `-m` are evaluated also when the
  Julia GC is running, but are their values really used?

- The introduction of the section "Command Line Options" refers to a file
  https://www.gap-system.org/Download/INSTALL which exists but seems to
  be outdated --which URL should be given here?
  (The corresponding file in the GAP distribution is `INSTALL.md`.)

- If GASMAN is not running, a file given with the `-L` command line option
  is currently ignored, because `LoadWorkspace` is not called when
  `GAP_ENABLE_SAVELOAD` is not defined.
  Should we better show an error message telling that no workspace
  could be loaded?
  (In earlier versions, there was such a message.)

- The function `GASMAN` is undocumented.
  (The Development manual contains a section "Garbage collection in GAP",
  where GASMAN is said to be "the GAP memory manager".
  This seems to be the right place to add documentation for alternative
  garbage collectors.
  On the other hand, this manual is not part of the GAP distribution.)

  Calling `GASMAN( "collect" )` and `GASMAN( "partial" )` work also
  with the Julia GC; if this is intentional then we should choose
  a better name for the function.
  (`GASMAN( "message" )` works only with the GASMAN GC.)

  The function `GASMAN` is shown in manual examples about weak pointer
  objects. Thus a function for forcing a garbage collection should be
  documented.

- The section "The GASMAN Interface for Weak Pointer Objects"
  in chapter "Weak Pointers" looks GASMAN specific (not only the title).
  How to reformulate it?
@ThomasBreuer ThomasBreuer added topic: documentation Issues and PRs related to documentation kind: discussion discussions, questions, requests for comments, and so on topic: julia Julia GC integration and related matters labels Nov 11, 2020
Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that's a great improvement! A few minor remarks

doc/ref/run.xml Outdated Show resolved Hide resolved
lib/system.g Outdated Show resolved Hide resolved
@fingolfin
Copy link
Member

  • The command line options -K and -m are evaluated also when the
    Julia GC is running, but are their values really used?

These set the C kernel variables SyStorKill resp. SyStorMin. The former is honored by GASMAN and Boehm GC, but not but by Julia GC. The latter is only honored by GASMAN.

The main reason I didn't limit them further in the GAP kernel is that their values are returned by GASMAN_LIMITS, which in turn is used by GasmanLimits. Perhaps they should simply be changed to return infinity when not used. Then we could disable them also in Julia GC resp. Boehm GC, on the kernel level...

  • The introduction of the section "Command Line Options" refers to a file
    https://www.gap-system.org/Download/INSTALL which exists but seems to
    be outdated --which URL should be given here?
    (The corresponding file in the GAP distribution is INSTALL.md.)

I left an inline comment on this.

  • If GASMAN is not running, a file given with the -L command line option
    is currently ignored, because LoadWorkspace is not called when
    GAP_ENABLE_SAVELOAD is not defined.
    Should we better show an error message telling that no workspace
    could be loaded?
    (In earlier versions, there was such a message.)

Agreed, showing an error message would be better.

  • The function GASMAN is undocumented.
    (The Development manual contains a section "Garbage collection in GAP",
    where GASMAN is said to be "the GAP memory manager".
    This seems to be the right place to add documentation for alternative
    garbage collectors.
    On the other hand, this manual is not part of the GAP distribution.)
    Calling GASMAN( "collect" ) and GASMAN( "partial" ) work also
    with the Julia GC; if this is intentional then we should choose
    a better name for the function.
    (GASMAN( "message" ) works only with the GASMAN GC.)
    The function GASMAN is shown in manual examples about weak pointer
    objects. Thus a function for forcing a garbage collection should be
    documented.

Indeed, there should be a function for garbage collection, with a neutral name, say CollectGarbage() which takes a boolean as argument. However, we should make it clear that in normal situation, there should be no need to invoke it.

Also, we still might want to mention something like "in older GAP versions, one can use GASMA("collect") for this"; or even something like:

The function CollectGarbage was introduced in GAP 4.12. If your code needs to be compatible with older GAP versions as well, you can use helper code like this:

if not CompareVersionNumbers( GAPInfo.Version, "4.12" ) then
  CollectGarbage := function(full)
   if full then
     GASMAN( "collect" );
   else
     GASMAN( "partial" );
   fi;
  end;
  • The section "The GASMAN Interface for Weak Pointer Objects"
    in chapter "Weak Pointers" looks GASMAN specific (not only the title).
    How to reformulate it?

That section IMHO simply does not belong into the reference manual, it should be in the dev manual instead.

- moved the Reference Manual section "The GASMAN Interface for Weak Pointer
  Objects" to the dev manual

- changed formulation "GASMAN is running" to "GAP uses GASMAN" in several places

- added a new documented function `CollectGarbage`,
  call this function instead of `GASMAN` in manual examples

- removed the link to `https://www.gap-system.org/Download/INSTALL`
  in the Reference Manual, since this file may not fit to the version of GAP

- added an error message if the `-L` command line option is given but the
  garbage collector is different from `GASMAN`
  (I did not find a good place for this check in the GAP kernel,
  because of the preprocessor flags.)
@fingolfin fingolfin added release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements labels Nov 26, 2020
@fingolfin fingolfin changed the title improve the documentation concerning GASMAN improve the documentation concerning GASMAN; add CollectGarbage Nov 26, 2020
Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but I'd appreciate if @ChrisJefferson could also have at least a quick peek at this.

@ChrisJefferson ChrisJefferson merged commit ea038eb into gap-system:master Nov 26, 2020
@ThomasBreuer ThomasBreuer deleted the TB_Julia_integration branch November 26, 2020 14:18
@ThomasBreuer ThomasBreuer self-assigned this Feb 16, 2021
@ThomasBreuer ThomasBreuer changed the title improve the documentation concerning GASMAN; add CollectGarbage Improved the documentation concerning GASMAN, added CollectGarbage. Feb 16, 2021
@ThomasBreuer ThomasBreuer added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Feb 16, 2021
@ThomasBreuer ThomasBreuer removed their assignment Feb 16, 2021
@fingolfin fingolfin changed the title Improved the documentation concerning GASMAN, added CollectGarbage. Improve the documentation concerning GASMAN, add CollectGarbage Aug 17, 2022
@fingolfin fingolfin removed the topic: julia Julia GC integration and related matters label Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: discussion discussions, questions, requests for comments, and so on kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: added PRs introducing changes that have since been mentioned in the release notes topic: documentation Issues and PRs related to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants