Project

General

Profile

Actions

Misc #14381

closed

[PATCH] ruby/ruby.h: remove unnecessary exports from C-API

Added by normalperson (Eric Wong) over 6 years ago. Updated over 6 years ago.


Description

Note this depends on r61995, will wait a few days for review
(ENOSPC on my end to run gem-codesearch)

Needlessly exporting can reduce performance locally and increase
binary size.

Increasing the footprint of our C-API larger is also detrimental
to our development as it encourages tighter coupling with our
internals; making it harder for us to preserve compatibility.

If some parts of the core codebase needs access to globals,
internal.h should be used instead of anything in include/ruby/*.

"Urabe, Shyouhei" <shyouhei@ruby-lang.org> wrote:
> On Thu, Jan 18, 2018 at 7:33 PM, Eric Wong <normalperson@yhbt.net> wrote:
> > shyouhei@ruby-lang.org wrote:
> >>   https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=61908
> >>
> >>     export rb_mFConst
> >
> > Why are we exporting all these and making the public C-API bigger?
> > If anything, we should make these static.  Thanks.
>
> No concrete reason, except they have already been externed in 2.5.
> These variables had lacked declarations so far, which resulted in their
> visibility to be that of extern. The commit is just confirming the status quo.
>
> I'm not against to turn them into static.

This reverts changes from r61910, r61909, r61908, r61907, and r61906.

* transcode.c (rb_eUndefinedConversionError): make static
  (rb_eInvalidByteSequenceError): ditto
  (rb_eConverterNotFoundError): ditto
* process.c (rb_mProcGID, rb_mProcUid, rb_mProcID_Syscall): ditto
* file.c (rb_mFConst): ditto
* error.c (rb_mWarning, rb_cWarningBuffer): ditto
* enumerator.c (rb_cLazy): ditto

Files

Updated by shyouhei (Shyouhei Urabe) over 6 years ago

Ack. Like I said I have no strong opinion on this.

Actions #2

Updated by Anonymous over 6 years ago

  • Status changed from Open to Closed

Applied in changeset trunk|r62029.


ruby/ruby.h: remove unnecessary exports from C-API

Needlessly exporting can reduce performance locally and increase
binary size.

Increasing the footprint of our C-API larger is also detrimental
to our development as it encourages tighter coupling with our
internals; making it harder for us to preserve compatibility.

If some parts of the core codebase needs access to globals,
internal.h should be used instead of anything in include/ruby/*.

"Urabe, Shyouhei" wrote:

On Thu, Jan 18, 2018 at 7:33 PM, Eric Wong wrote:

wrote:

https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=61908

export rb_mFConst

Why are we exporting all these and making the public C-API bigger?
If anything, we should make these static. Thanks.

No concrete reason, except they have already been externed in 2.5.
These variables had lacked declarations so far, which resulted in their
visibility to be that of extern. The commit is just confirming the status quo.

I'm not against to turn them into static.

This reverts changes from r61910, r61909, r61908, r61907, and r61906.

  • transcode.c (rb_eUndefinedConversionError): make static
    (rb_eInvalidByteSequenceError): ditto
    (rb_eConverterNotFoundError): ditto
  • process.c (rb_mProcGID, rb_mProcUid, rb_mProcID_Syscall): ditto
  • file.c (rb_mFConst): ditto
  • error.c (rb_mWarning, rb_cWarningBuffer): ditto
  • enumerator.c (rb_cLazy): ditto
    [Misc #14381]
Actions

Also available in: Atom PDF

Like0
Like0Like0